fix DCDWriter angle cosines #5071
Conversation
- fix #5069 - DCDWriter is now writing unit cell angles as cos(angle) to the DCD file as written in the docs (NAMD >= 2.5 convention) - add test - update CHANGELOG
|
FYI: I tried out AI assisted coding with Cursor, using the default model:
I rewrote basically everything; the only key thing that survived was the idea to write the cosines as sin(90 - angle), which it picked up from reading the DCDReader code. For the tests, it did not properly use my suggestions from the issue report to use the DCDFrame that the underlying dcd reading code provides. Still, it was helpful as a starting point. It had a really hard time formatting the CHANGELOG entry, trying 5 different ways, finally writing a sed script to do it, ... poor thing. (It also tried to delete half of CHANGELOG in the process....) |
- Original test only passed at rtol=1e-4 due to some angles close to 0, which is where the sin(90-angle) storage is much less accurate. Changed the test range of angles to between 5 and 120 (from 0 to 80) to be more realistic and exclude very small angles where we would see precision issues - formatted with black
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #5071 +/- ##
========================================
Coverage 93.62% 93.62%
========================================
Files 177 177
Lines 22001 22002 +1
Branches 3114 3114
========================================
+ Hits 20599 20600 +1
Misses 947 947
Partials 455 455 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Fixes #5069
Changes made in this Pull Request:
PR Checklist
package/CHANGELOGfile updated?package/AUTHORS? (If it is not, add it!)Developers Certificate of Origin
I certify that I can submit this code contribution as described in the Developer Certificate of Origin, under the MDAnalysis LICENSE.
📚 Documentation preview 📚: https://mdanalysis--5071.org.readthedocs.build/en/5071/