Enable kDTree optimization for PCFAngle and PCF2d#159
Enable kDTree optimization for PCFAngle and PCF2d#159UrbanGalaxy wants to merge 11 commits intoamepproject:leifs_editsfrom
Conversation
There was a problem hiding this comment.
please do not add your personal .gitignore lines here.
you may push .vscode/ as it is a common directory but not your personal file structure please.
| # # -> 'ni' = (N particles, 3 rotated coords) | ||
| # coords = np.einsum('nij,nj->ni', R, coords) | ||
|
|
||
| # # redo the shift operation | ||
| # coords = coords + center | ||
| # return coords |
There was a problem hiding this comment.
please clean code and remove comments
There was a problem hiding this comment.
why have you created this file?
please check how tests are done properly. a minimal run of a functionality which is compared on the data level and not comparing two saved files (which would be a test on its own)
There was a problem hiding this comment.
please add any test to the test suite in test/test_evaluate.py for example. or test/test_spatialcor.py
| if not os.path.exists(plot_dir): | ||
| os.makedirs(plot_dir) | ||
|
|
||
| print("AAAAA") |
|
|
||
| def rotate_coords( | ||
| coords: np.ndarray, theta: float, center: np.ndarray) -> np.ndarray: | ||
| coords: np.ndarray, theta: float | np.float32 | np.float64, center: np.ndarray) -> np.ndarray: |
There was a problem hiding this comment.
please remove np.float32 and np.float64. these should not be used for type hinting here.
There was a problem hiding this comment.
Why this modification of the test workflow?
There was a problem hiding this comment.
so the test run when something is pushed, this way i was able to see that is passed the tests without initiating the pull request
There was a problem hiding this comment.
even though this sounds nice, this is not the aimed process. If you want to check the tests on GitHub, a pull request can be created.
kay-ro
left a comment
There was a problem hiding this comment.
Please address the comments. Afterwards, I can check the functionality.
|
I think i changed everything you requested besides my modification to the test workflow. If you want me to change it or anything else let me know. |
I added a use_kDTree boolean parameter and the corresponding logic to PCFAngle and PCF2d. This allows the calculation to use a kDTree nearest-neighbor list. I also added the orientation mode for PCF2d.
Motivation/Benefits: Previously, the pair correlation was calculated over all particles. By using the kDTree, the calculation is now restricted to relevant particles within the cutoff distance (r≤rmax), which significantly improves performance.