Adding subtract self to the neighborlist generator #19
Conversation
This file contains 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
|
This PR may rely on #20 before it is clean to merge |
rosecers
force-pushed
the
feat/respect_self_arg
branch
from
020b8e1 to
4f54460
Compare
|
@arthur-lin1027 or @acrowley5 can you take a look? |
|
Reminder @arthur-lin1027 @acrowley5 to review this |
YCC-ProjBackups
approved these changes
Nov 10, 2023
There was a problem hiding this comment.
I looked through this code, and ran my own test script, and it seems to be working fine. I also checked multiple times whether putting not in front of the self.subtract_center_contribution is necessary, and I am quite confident that putting not in front leads to the correct and intended implementation.
The parenthesis around not self.subtract_center_contribution is not necessary, but since it will have literally zero impact, I decided to leave it alone.
I will mark this PR as "Approve" for now, but let me know if you want me to do more in-depth analysis (though you may have to be specific, as I don't know what else I need to do)
ortengren
pushed a commit
that referenced
this pull request
Nov 22, 2023
rosecers
pushed a commit
that referenced
this pull request
Dec 5, 2023
Implemented Rust FFI for moments and caching for CGR. "Committing Rust FFI-related files" Changed name of csv file to something shorter. Changed the file system so that the file can be run from the root with python3 -m tests.ex_input Moved all configs to ex_inputs.py file and ran diff check on results from two languages. Implemented max-16-iter rule check for time mode Fixed a bug that occurred when code_timer in ex_input.py was set to None. Deleted unnecessary folder, equistore. Changed the script to perform comparison between different versions Updating isort on __init__ Update tests.yml to include coverage (#8) Update tests.yml Update tests.yml Update tests.yml Update README.md Update tests.yml (#9) * Update tests.yml * Update tests.yml Update tests.yml (#9) * Update tests.yml * Update tests.yml Adding new tests for EDP (#7) * Adding new tests for EDP * Making requisite changes for equistore compatibility * improved code coverage to 99% by testing for 3 additional cases: show_progress=True, multiple frames, and matrix rotations * pass the linter --------- Co-authored-by: Arthur Lin <[email protected]> 5 incorporate normalization factors (#6) * Added (and made default behavior) the ability to orthonormalize features that use the GTO basis. * This involves normalizing the features properly, creating an overlap matrix with orthogonal GTOs, and orthogonalizing the features. * Added relevant tests to test new orthonormality functionality * Added a jupyter notebook displaying how Lowdin Orthonormalization works (on a small gto basis set). --------- Co-authored-by: Rose K. Cersonsky <[email protected]> Co-authored-by: Arthur Lin <[email protected]> minor changes to accomodate new equistore api Updating to be in line with new equistore formatting (#15) Adding progress bar for sanity sake (#14) added warning for passing in integer, and cast any int arguments to f… (#13) * Raise an error when a float is passed for radial_gaussian_width, and added a unit test to ensure error is raised --------- Co-authored-by: Arthur Added code that allows multiple iterations per parameter set. Now uses tqdm to keep track of progress. Removed all internal timing code. The version selection feature still remains for testing purposes. Implemented v2, in which loop for pairwise_ellip_expansion changed (further testing needed). Also, minor changes to output file. Cross-platform support? Needs further testing Added mac move command Fixed file extension error on Mac Moved caching logic to CGR class itself, instead of passing it as a keyword argument. Removed unnecessary keyword argument from single_pass function minor syntax changes for 3.9 added test ellipsoid trimers for performance testing Added subprocess shell command to automatically run makefile. Running makefile automatically while pip install version 2. Added ell-trimers.xyz file. Cross platform support... hopefully. Version 3. Added MANIFEST.in file Changed l_cut to lcut in cg_combine. Fixed the issue of v0's output differing from the original implementation. Some minor cleanup before some more experimental changes Some code cleanup, mostly on the Rust side. Updated equistore to metatensor. Requires changing _classes.py of metatensor. Updated metatensor and rascaline to latest version. Changed import statements, and no _classes.py modification necessary. Removed unused (for now) Rust dependencies Some basic modification for test. Added code to test running time depending on frame numbers. Added code to test running time depending on frame numbers. Changed implementation to determine repetition number with argv Changed CGRCacheList replacement technique from FIFO to Clock (and added documentation) Fixed a detail in Clock algorithm (initial insertion should have replacement flag = 0) Fixed uninitialized _keys in cyclic_list.py Fixed uninitialized _keys in cyclic_list.py Updated metatensor field names to the most recent changes Removing equistore in favor of metatensor (#20) * Removing equistore in favor of metatensor * fixing metatensor.core Adding subtract self to the neighborlist generator (#19) Switching list comp (#24) Switching the code to do the list comp once instead of within each loop Fixed mismatch between Python and Rust compute_moment Cleaned up unused imports and variables. Fixed minor spelling error Deleted unncessary space in monomial_iterator.py Ran isort to satisfy the linter. Changed Rust implementation such that it takes inverse and determinant from Numpy instead of calculating internally. Deleted version control switches and ran isort to satisfy linter. Ran isort again. Fixed a bug that occurs when both error AND inverse are 0. Copied main's contract_pairwise_feat code. Adding ability to override number of radial bases manual override n and added test cases removed extraneous arg Linter Linter Pointing to cutoff radius properly I messed with the history so now I'm restoring it pass linter removed error check restricting independent specification of max_radial and radial_gaussian_width added tests to ensure constant n functionality is correct linter num_radial -> max_radial in edp.py removed changes pertaining to coupling sigma and n to keep PR clean restored original compute_gaussian_parameters function header incorporated necessary changes to make constant n work for both orthonormalization and moments generation pass linter removed deprecated comment made definition of maxradial consistent with num_n and updated tests Update lint.yml (#25) add diff to linter so we can see how files are failing the linter. Used Black formatter
ortengren
added a commit
that referenced
this pull request
Jun 6, 2024
* Configured Sphinx and reformatted docstrings * Adding subtract self to the neighborlist generator (#19) * Switching list comp (#24) Switching the code to do the list comp once instead of within each loop * Adding ability to override number of radial bases * manual override n and added test cases * removed extraneous arg * Linter * Linter * Pointing to cutoff radius properly * I messed with the history so now I'm restoring it * pass linter * removed error check restricting independent specification of max_radial and radial_gaussian_width * added tests to ensure constant n functionality is correct * linter * num_radial -> max_radial in edp.py * removed changes pertaining to coupling sigma and n to keep PR clean * restored original compute_gaussian_parameters function header * incorporated necessary changes to make constant n work for both orthonormalization and moments generation * pass linter * removed deprecated comment * made definition of maxradial consistent with num_n and updated tests * Update lint.yml (#25) add diff to linter so we can see how files are failing the linter. * Resolved merge conflict by incorporating both suggestions * formatted docstrings, adjusted Sphinx config * Fixed docstrings, adjusted sphinx config, manually rewrote modules.rst * Fixed docstring formatting and changed sphinx theme * Added custom CSS, replaced modules.rst with api.rst, adjusted sphinx conf and index.rst * added ReadTheDocs configuration * Fixed requirements.txt for RTD building * Fixed requirements.txt * Removed anisoap from requirements.txt * removed unneccessary packages from requirements.txt * changed config to not fail upon warning * edited RTD config and requirements.txt * changed requirements.txt to install metatensor and rascaline * Fixing requirements.txt * Fixed requirements.txt * fixed requirements.txt * fixed requirements.txt * Changed RTD config * fixed rust version in RTD config * fixed rust version in RTD config * changed pytorch requirement to cpu version * fixed torch dependency * edited metatensor dependency * added tqdm to requirements.txt * deleted modules.rst * fixed api.rst * fixed autosummary in api.rst * fixed autodocs in api.rst * Changed api.rst to have more detailed description * further formatted docstrings * fixed mathjax formatting * fixed mathjax formatting * restructuring documentation * Fixing LaTeX and docstring formatting * Changing copyright info, author info * Removing .idea folder * Building docs * Built HTML and fixed docstrings * linter * isort * linter * deleted anisoap/reference/projection_coefficients.py * Removed documentation for deleted module * Fixed RST syntax errors in docstrings * Formatted according to Black code style * Delete anisoap/.idea directory These seem ide specific, don't want them being on the anisoap main branch. * equistore --> metatensor * minor correction --------- Co-authored-by: Rose K. Cersonsky <[email protected]> Co-authored-by: Arthur Lin <[email protected]> Co-authored-by: arthur-lin1027 <[email protected]>
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.
No description provided.