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.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bump
monty
to use themonty.json
import speedup patch, add import time regression test, lazy load some rarely used but costly modules #4128bump
monty
to use themonty.json
import speedup patch, add import time regression test, lazy load some rarely used but costly modules #4128Changes from 34 commits
d2447ed
1eb855b
aae9f4b
e08454e
f8b5723
1948442
03df9b0
4492816
a123174
9688244
6e0a89c
75e23b1
4320868
77ebf2a
206ea52
25975dd
762b85a
038ce9a
24fdcdc
022b91a
3dd2cdf
9b7f35a
87267f4
3435e99
aa4153a
313d553
e69e344
fa17222
74ec0c9
df8e2ff
2778e28
8952c4a
c37ba45
1674a96
8ac0c7f
2f7f993
abf5f8b
de4dc3b
8f5c2ac
4849da0
aceb5b5
eb51fee
c64ecae
54752f7
9c3d186
9ea93eb
1589f39
b001361
a5ff2a5
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
matplotlib
is likely not a core module for pymatgen, and incurred noticeable overhead tocore.interface
:After:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
np.testing.assert_allclose
is not suitable for production code (~ 2.5x slower thannp.allclose
), though it provides more detailed debug info for testing.Note the default tolerances are a bit stricter for
assert_allclose
, this commit also migrated the tolerances as is.Test script:
I got:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
math.isclose
is much faster (~ 500x/ ~ 188x) for comparing single scalar thanassert_allclose
ornp.isclose
.Gives:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AseAtomsAdaptor
is only used in one of the many try-except branches (other branches also lazy import the corresponding modules), ~10% speed up.pymatgen/src/pymatgen/core/trajectory.py
Lines 569 to 593 in 6992aee