-
Notifications
You must be signed in to change notification settings - Fork 12
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
Update autode to 1.3.3 #121
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #121 +/- ##
==========================================
- Coverage 64.99% 64.90% -0.09%
==========================================
Files 37 37
Lines 4268 4280 +12
==========================================
+ Hits 2774 2778 +4
- Misses 1494 1502 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Error observed when loading mg_aqua_mace_al.npz file (prepared with autode1.1.3) using autode1.4.4
|
|
Error observed with using autode v1.1.3 to 1.3.3, most probably data set specific:
|
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.
Looks great, just some minor comments.
.pre-commit-config.yaml
Outdated
@@ -11,7 +11,7 @@ repos: | |||
exclude: examples/WTMetaD_paper/r2/free_energy/wtmetad_ib/accumulated_bias/bias_after_iter_15.dat | |||
- id: check-added-large-files | |||
args: ['--maxkb=500', '--enforce-all'] | |||
exclude: tests/data/data.zip | |||
exclude: (?x)^(tests/data/data.zip | tests/data/water_al.npz)$ |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
if data['R_plumed'].ndim > 0: | ||
config.plumed_coordinates = np.array( | ||
data['R_plumed'][i], dtype=float | ||
try: |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
@@ -137,6 +139,41 @@ def test_configurations_load_with_energies_forces(): | |||
) | |||
|
|||
|
|||
@work_in_tmp_dir() | |||
def test_configurations_load_with_energies_forces_diff_sizes( |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
tests/test_configuration_set.py
Outdated
for attr in ('energy', 'forces'): | ||
for kind in ('predicted', 'true'): | ||
assert np.allclose( | ||
getattr(getattr(loaded_configs[0], attr), kind), |
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.
This is clever but hard to read. I would trade the for loops for a bit of (straightforward) code repetition.
@@ -161,6 +198,19 @@ def test_configurations_load_xyz(): | |||
assert config.mult == 2 | |||
|
|||
|
|||
def test_configurations_load_numpy_compatibility(): |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The next investigation step here, ideally before merging, would be to try to minimize the |
Co-authored-by: Daniel Hollas <danekhollas@gmail.com>
Co-authored-by: Daniel Hollas <danekhollas@gmail.com>
Co-authored-by: Daniel Hollas <danekhollas@gmail.com>
Co-authored-by: Daniel Hollas <danekhollas@gmail.com>
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.
Nice, what an epic debugging. I'll open an issue related to autode=1.3.4 and loading of old npz files.
Preliminary update to autode v1.3.3. Update to the latest version will continue once I figure out the source of npz loading issue which started to occur in v1.3.4
Also closes #127