Repository navigation
Reject negative unit-cell lengths in Timestep - #5537
swf1996120 wants to merge 1 commit into
Conversation
Documentation build overview
|
03aaed8 to
d5e73e0
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #5537 +/- ##
===========================================
- Coverage 93.92% 93.91% -0.01%
===========================================
Files 184 184
Lines 22661 22661
Branches 3223 3223
===========================================
- Hits 21284 21283 -1
- Misses 917 918 +1
Partials 460 460 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
IAlibay
left a comment
There was a problem hiding this comment.
@swf1996120 - we require everyone to fill in the PR template especially the AI use section and having the developer certificate of origin declaration.
The former is made clear in our AI policy: https://github.com/MDAnalysis/mdanalysis/blob/develop/AI_POLICY.md#0-ai-usage-must-always-be-declared
Given this failure to follow policy, and your excessive pull requests across the open source ecosystem (e.g. openmm/openmm#5559 (comment)), I am going to block and close this PR.
Problem
This fixes #5536.
Timestep.dimensionsaccepted a negative unit-cell length. The stored dimensions then produced a zero triclinic matrix and zero volume, silently turning invalid periodic geometry into a degenerate box.Fix
Normalize the input to a six-element float array and reject negative values in the first three length fields. The existing all-zero no-unit-cell sentinel remains supported.
Tests
Added regression coverage for negative finite and negative-infinite lengths. Existing zero-box behavior remains unchanged.
This was found during a chemical-repository testing study using physical unit-cell validity as the domain-specific invariant.