fix DCDWriter angle cosines

[orbeckst]

Fixes #5069

Changes made in this Pull Request:

• 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

PR Checklist

• Issue raised/referenced?
• Tests updated/added?
• Documentation updated/added?
• package/CHANGELOG file updated?
• Is your name in 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/

[orbeckst]

FYI: I tried out AI assisted coding with Cursor, using the default model:

Look at issue @https://github.com/MDAnalysis/mdanalysis/issues/5069 . Propose a fix to DCDWriter that addresses the problem of incorrectly writing unitcell angles to the DCD file. Propose tests (see @test_dcd.py ).

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....)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.85%. Comparing base (b4ab7e5) to head (9548987).
⚠️ Report is 5 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #5071   +/-   ##
========================================
  Coverage    93.85%   93.85%           
========================================
  Files          178      178           
  Lines        22122    22123    +1     
  Branches      3129     3129           
========================================
+ Hits         20763    20764    +1     
  Misses         902      902           
  Partials       457      457           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[orbeckst]

@p-j-smith or @yuxuanzhuang would you be able to have a look at this fairly simple bug fix in the DCDWriter code. please?

Alternatively, do you have other suggestions for reviewers?

[IAlibay]

Reading this, it sounds to me like we are replacing one feature with another, not necessarily fixing a "bug".

I get that our docs were wrong, but I wonder how many folks downstream expected charmm format instead of named/vmd. Do we think maybe putting in a flag to choose between the two might be useful? Either way, some docs that can loudly say "this is a change" might be useful.

[orbeckst]

Thanks for chiming in.

CHARMM has been using box vectors for a while, which is why we have this code :

mdanalysis/package/MDAnalysis/coordinates/DCD.py

Lines 248 to 252 in d412c9a

	elif np.any(uc < 0.) or np.any(uc[3:] > 180.):
	# might be new CHARMM: box matrix vectors
	H = frame.unitcell.copy()
	e1, e2, e3 = H[[0, 1, 3]], H[[1, 2, 4]], H[[3, 4, 5]]
	uc = triclinic_box(e1, e2, e3)

To my understanding, nobody should be writing the bare unit cell info any more because that creates just more format confusion.

I am happy to add a warning box to the docs although I'd rather not add more confusion by making it possible to switch between old and new NAMD conventions.

[orbeckst]

I added docs warning.

[orbeckst]

@IAlibay is the warning in the docs sufficient or do you just think we shouldn't be changing anything?

[IAlibay]

I think it's ok since the reader should still work either way. It's not ideal but I can see how this could be a bugfix.

[orbeckst]

I ran black (v24.10.0) on DCD.py and it made more changes than in the code that I touched. I was a bit surprised because I assumed that this file had originally been reformatted a while ago already. Sorry for the noisy diff – I can undo if this is preferred.

[RMeli]

I ran black (v24.10.0) on DCD.py and it made more changes than in the code that I touched.

@orbeckst did you run it directly on the file? The DCD file is one of the excluded ones

mdanalysis/package/pyproject.toml

Line 157 in b4ab7e5

| MDAnalysis/coordinates/DCD\.py

The exclusion list was added to minimize the disturbance to open PRs, but probably it is time to update it.

[orbeckst]

I reverted black – probably should be done separately.