Add cumulative strain guard to FixSymmetry #447
Merged
+178
−32
Conversation
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
Store initial refined cells as reference_cells and check cumulative strain in adjust_cell(). When total deformation exceeds max_cumulative_strain (default 0.5), clamp the cell update to stay within the strain envelope. Also clamp (instead of reject) per-step deformations > 0.25 so the optimizer can still make progress. Propagate reference_cells through reindex(), merge(), select_constraint(), and select_sub_constraint().
- Raise RuntimeError on NaN/Inf deformation gradient instead of silently propagating (the raise→clamp refactor broke the old NaN-catching `not (x <= 0.25)` idiom) - Tighten test_large_deformation_clamped bound from 0.5 to 0.25 (was asserting cumulative limit, not per-step limit) - Validate reference_cells length in __init__ - Add max_cumulative_strain to merge() validation - Fix stale docstring and misleading test comment
- Test NaN deformation raises RuntimeError (mutation-verified) - Test mismatched reference_cells length raises ValueError - Use math.isfinite() instead of isnan()/isinf() - Inline single-use clamped_strain variable
janosh
force-pushed
the
cumulative-strain-guard
branch
4 times, most recently
from
0dd5822 to
a7198f4
Compare
mattersim and fairchem (via torchtnt) import pkg_resources at module level. Python 3.12 no longer bundles setuptools by default, and recent GitHub runner image updates removed it.
janosh
force-pushed
the
cumulative-strain-guard
branch
from
a7198f4 to
5bd271f
Compare
Collaborator
|
This looks like a nice change @janosh. IMO in general it'd be best to get a review on non-trivial changes before merging in. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
reference_cellsinfrom_state()and track cumulative strain inadjust_cell()max_cumulative_strain(default 0.5), clamp the cell update to stay within the strain envelopereference_cellsthroughreindex(),merge(),select_constraint(), andselect_sub_constraint()Builds on #438 (FixSymmetry constraint). The per-step deformation check (
max_delta <= 0.25) catches large single-step cell changes but not slow cumulative drift across many steps that can cause phase transitions (e.g. hexagonal → tetragonal cell collapse).Complementary to #443 which fixes L-BFGS history contamination from unsymmetrized forces — that PR addresses force-direction drift, this one addresses cell-deformation drift.
Test plan
test_large_deformation_clampedupdated: verifies per-step clamping and cumulative strain boundtest_cumulative_strain_clamp_direct: 20 small anisotropic stretches that individually pass per-step check but cumulatively exceed the limit; verifies clamping and symmetry preservation