Transformed cube (#15)#96
Open
Ali-Tehrani wants to merge 47 commits into
Open
Conversation
Added the transformation from real space to unit cube using the Promolecular density function of a single coordinate. Added the inverse transformation from unit-cube to real space using a root-solver of a single coordinate.
* Promolecular Cubic Grid Transformation is added. * Bracket initalization is added as private func. * Padding multiple arrays with zeros is added st have array size. * Full Grid transformation from unit-cube to real space.
Need this for integration
Also added the promolecular trick.
Follows changes were made - Added and Fixed docuentation. - Added jacobian of transformation - Added steepest-ascent - Added utility to transform individual points
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #96 +/- ##
===========================================
- Coverage 100.00% 99.12% -0.88%
===========================================
Files 14 14
Lines 1385 1592 +207
===========================================
+ Hits 1385 1578 +193
- Misses 0 14 +14 ☔ View full report in Codecov by Sentry. |
Private data class that holds the coefficients, exponents, dimensions and compute its density.
From code review, it was not necessary.
Providing number of points seems more natural than providing the stepsize for Promolecular Cubic, Uniform Grid Transformation.
Code review suggestion to use masked arrays for boundary points and small values of promolecular with a user-defined tolerance.
Suggested in code review, able to handle insufficient brackets.
Cubic grid is a tensor product of one-dimensional grids. Promolecular transform is modeled on a cubic grid.
Since the bounds [-1, 1] is used in onedgrids.py, the default bounds is changed to be consistent.
Since cubic grids are not supported, I tested integration with GaussChebslev oned grid.
Added - convert index to indices to grid transform - convert indices to index to grid transform - interpolation and its derivative - hessian
dataclass is included in python3.7. Acknowledgement to Derrick
Also removed an if statement that wasnt really needed and to improve coverage
- Makes it easier for viewing and generalizes some test to work over a wider range of points
- Makes it consistent for the cubic grid class
Useful for promolecular transformation
- Useful for promolecular transform where the grid to interpolate is different - Make inteprolate its own function due to problems with sub-classing in python
- Needed to say a point is on the boundary
- Increases generality
- Matches the HyperRectangle class
- For promolecular transform
Contributor
Author
|
Added new changes:
|
Member
|
This is not urgent, but in the interest of getting it (eventually) merged, I've assigned @marco-2023 to it. @marco-2023 we can talk about this at some point. One nice feature of this grid is that it is easy to adapt, at least in principle. The easiest way to iteratively refine the grid is to take each grid point and identify its closest neighbors. The value of This would need to be thought through carefully, but this is the basic idea. It is a reasonably efficient procedure once an initial kd-tree is constructed, I think. |
This was referenced Feb 9, 2025
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new promolecular transformation for cubic grids, enabling mapping between theta-space [-1, 1]^3 and real space R^3, along with Jacobian/Hessian utilities and accompanying tests.
Changes:
- Introduces
CubicProTransform(promolecular grid transform) with transform/inverse, integration, Jacobian/Hessian, and interpolation support. - Extends cubic-grid interpolation internals to accept optional explicit grid point coordinates (
grid_pts) to support promolecular theta grids. - Adds a large new test suite for promolecular transform behavior; fixes a typo in an existing cubic interpolation test name; updates dependencies.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
src/grid/protransform.py |
New promolecular cubic-grid transformation implementation (transform/inverse, integrate, jacobian/hessian, interpolation). |
src/grid/cubic.py |
Adds grid_pts plumbing and refactors interpolation to avoid subclass recursion issues. |
src/grid/tests/test_protransform.py |
New tests for promolecular density, transforms, derivatives, interpolation, and integration. |
src/grid/tests/test_cubic.py |
Fixes typo in a test name. |
pyproject.toml |
Adds dataclasses dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+974
to
+991
| f_l_bnd = _root_equation(l_bnd, *args) | ||
| f_u_bnd = _root_equation(u_bnd, *args) | ||
|
|
||
| # Get Index of the one that is closest to zero, the one that needs to change. | ||
| f_bnds = np.abs([f_l_bnd, f_u_bnd]) | ||
| idx = f_bnds.argmin() | ||
| # Check if they have the same sign. | ||
| same_sign = is_same_sign(*bounds) | ||
| counter = 0 | ||
| while same_sign and counter < maxiter: | ||
| # Add 10 to the upper bound or subtract 10 to the lower bound to the one that | ||
| # is closest to zero. This is done based on the sign. | ||
| bounds[idx] = np.sign(idx - 0.5) * 10 + bracket[idx] | ||
| # Update info for next iteration. | ||
| if idx == 0: | ||
| f_l_bnd = _root_equation(bracket[0], *args) | ||
| else: | ||
| f_u_bnd = _root_equation(bracket[1], *args) |
Comment on lines
+24
to
+29
| from grid.basegrid import Grid, OneDGrid | ||
| from grid.cubic import _HyperRectangleGrid | ||
|
|
||
| import numpy as np | ||
|
|
||
| from scipy.interpolate import CubicSpline |
| interpolation = np.vstack((interpolate_x.T, interpolate_y.T, interpolate_z.T)).T | ||
| # Transform the derivatives back to real-space. | ||
| interpolation = np.array( | ||
| [self.jacobian(points[i]).dot(interpolation[i]) for i in range(len(interpolation))] |
Comment on lines
+306
to
+316
| integrands = [] | ||
| for arr in value_arrays: | ||
| # This is needed as it gives incorrect results when arr.dtype isn't object. | ||
| assert arr.dtype != object, "Array dtype should not be object." | ||
| # This may be refactored to fit in the general promolecular trick in `grid`. | ||
| # Masked array is needed since division by promolecular contains nan. | ||
| if trick: | ||
| integrand = np.ma.divide(arr - promolecular, promolecular) | ||
| else: | ||
| integrand = np.ma.divide(arr, promolecular) | ||
| # Function/Integrand evaluated at points on the boundary is set to zero. |
Comment on lines
+247
to
+253
| - If a point is far away from the promolecular density, then it will be mapped | ||
| to `np.nan`. | ||
|
|
||
| """ | ||
| real_pt = [] | ||
| for i in range(0, self.promol.dim): | ||
| scalar = _inverse_coordinate(theta_pt[i], i, real_pt[:i], self.promol, bracket) |
Comment on lines
+139
to
+142
| grid_pts: list[OneDGrids], optional | ||
| If provided, then uses `grid_pts` rather than the points of the HyperRectangle class | ||
| `self.points` to construct interpolation. Useful when doing a promolecular | ||
| transformation. |
| "pytest>=8.0.0", | ||
| "scipy>=1.4", | ||
| "importlib_resources", | ||
| "dataclasses", |
Comment on lines
+474
to
+513
| @pytest.mark.parametrize("pts", [np.arange(-5.0, 5.0, 0.5)]) | ||
| def test_transforming_x_against_numerics(self, pts): | ||
| r"""Test transforming X against numerical algorithms.""" | ||
| for pt in pts: | ||
| true_ans = _transform_coordinate([pt], 0, self.setUp()) | ||
|
|
||
| def promolecular_in_x(grid, every_grid): | ||
| r"""Construct the formula of promolecular for integration.""" | ||
| promol_x = 5.0 * np.exp(-2.0 * (grid - 1.0) ** 2.0) | ||
| promol_x_all = 5.0 * np.exp(-2.0 * (every_grid - 1.0) ** 2.0) | ||
| return promol_x, promol_x_all | ||
|
|
||
| grid = np.arange(-4.0, pt, 0.000005) # Integration till a x point | ||
| every_grid = np.arange(-5.0, 10.0, 0.00001) # Full Integration | ||
| promol_x, promol_x_all = promolecular_in_x(grid, every_grid) | ||
|
|
||
| # Integration over y and z cancel out from numerator and denominator. | ||
| actual = -1.0 + 2.0 * np.trapz(promol_x, grid) / np.trapz(promol_x_all, every_grid) | ||
| assert np.abs(true_ans - actual) < 1e-5 | ||
|
|
||
| @pytest.mark.parametrize("pts_xy", [np.random.uniform(-10.0, 10.0, size=(100, 2))]) | ||
| def test_transforming_y_against_numerics(self, pts_xy): | ||
| r"""Test transformation y against numerical algorithms.""" | ||
|
|
||
| def promolecular_in_y(grid, every_grid): | ||
| r"""Construct the formula of promolecular for integration.""" | ||
| promol_y = 5.0 * np.exp(-2.0 * (grid - 2.0) ** 2.0) | ||
| promol_y_all = 5.0 * np.exp(-2.0 * (every_grid - 2.0) ** 2.0) | ||
| return promol_y_all, promol_y | ||
|
|
||
| for x, y in pts_xy: | ||
| true_ans = _transform_coordinate([x, y], 1, self.setUp()) | ||
|
|
||
| grid = np.arange(-5.0, y, 0.000001) # Integration till a x point | ||
| every_grid = np.arange(-5.0, 10.0, 0.00001) # Full Integration | ||
| promol_y_all, promol_y = promolecular_in_y(grid, every_grid) | ||
|
|
||
| # Integration over z cancel out from numerator and denominator. | ||
| # Further, gaussian at a point does too. | ||
| actual = -1.0 + 2.0 * np.trapz(promol_y, grid) / np.trapz(promol_y_all, every_grid) |
Comment on lines
+703
to
+705
| min = np.min(self.promol.coords[:, i_var]) - 20.0 | ||
| max = np.max(self.promol.coords[:, i_var]) + 20.0 | ||
| return min, max |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Promolecular Transformation of a Cubic grid class. This is regarding issue #15.
Functionality
TODO
May 16, 2023. All of the TODO except for the "Directional derivative tests" are now fixed/added.