Fix parsing of box vector in H5MD reader

[schlaicha]

Fixes #4075

Changes made in this Pull Request:

• Allow for vector box edges in H5MD reader

PR Checklist

• Tests?
• Docs?
• CHANGELOG updated?
• Issue raised/referenced?

---

📚 Documentation preview 📚: https://readthedocs-preview--4076.org.readthedocs.build/en/4076/

[codecov]

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (09b8b16) 93.56% compared to head (64ac2e4) 93.57%.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #4076   +/-   ##
========================================
  Coverage    93.56%   93.57%           
========================================
  Files          191      191           
  Lines        25083    25086    +3     
  Branches      4050     4051    +1     
========================================
+ Hits         23470    23473    +3     
  Misses        1093     1093           
  Partials       520      520           

Impacted Files	Coverage Δ
package/MDAnalysis/coordinates/H5MD.py	93.12% <100.00%> (+0.04%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[orbeckst]

@edisj would you be able to have a quick look at this PR and provide a review?

[edisj]

@edisj would you be able to have a quick look at this PR and provide a review?

From a glance it looks good, only thing to add would be a unit test. I will take a more thorough look over the weekend

[orbeckst]

I'll let @edisj look at the actual content of the PR. I am just requesting some formalities that we ask for all first time contributors

• Please add yourself to AUTHORS
• I added your GH handle to CHANGELOG
• please introduce yourself on the developer list https://groups.google.com/group/mdnalysis-devel — just say hello and mention the PR you're working on

On the doc side, please add .. versionchanged:: 2.5.0 to the H5MDReader docs, indicating that simple cuboid boxes can now be correctly read.

Thank you for your contribution, @schlaicha , much appreciated!

[edisj]

hi @schlaicha, thanks for bringing this up! I definitely missed the cuboid box handling when I wrote this a couple years ago. Out of curiosity, do you regularly use the h5md format in your research? And are there other programs that you know of that regularly use h5md? I'd be very happy to hear h5md in mdanalysis is used regularly somewhere :).

Your fix LGTM, I'm only requesting that you add a unit test in https://github.com/MDAnalysis/mdanalysis/blob/develop/testsuite/MDAnalysisTests/coordinates/test_h5md.py#L344, I wrote a simple example test that should do the job below that creates a dummy file with 3-D vectors for its box dimensions. Please feel free to write your own test, though

    def test_box_vector(self, h5md_file, outfile):
        with h5md_file as f:
            with h5py.File(outfile, 'w') as g:
                f.copy(source='particles', dest=g)
                f.copy(source='h5md', dest=g)
                vector = [1, 2, 3]
                del g['particles/trajectory/box/edges/value']
                g['particles/trajectory/box/edges/value'] = [vector, vector, vector]
        u = mda.Universe(TPR_xvf, outfile)
        # values in vector are conveted from nm -> Angstrom
        assert_equal(u.trajectory.ts.dimensions, [10, 20, 30, 90, 90, 90])

@orbeckst should we also change the behavior in H5MDWriter in this PR? It currently assumes a shape of (n_frames, 3, 3) for box edges in the h5md file. It still technically works because it just creates a diagonal matrix for the cuboid case, but it's not exactly following the h5md standard.

[schlaicha]

Thank you for your feedback!

I was about to add a test with the trajectory submitted ini the bug report, but I like your approach much better.
I have added the test but don't get the issue with linter error?

So far xtc was my preferred trajectory format but I regularly need velocities and forces too, so I guess I will become a more frequent user of H5MD for simulations in LAMMPS and ESPResSo...

I read the H5MD standard a bit differently, it is perfectly fine to have a cubic box written as a matrix in my opinion. So I do not see a need to change the writer.

[orbeckst]

Thanks @edisj . I don't think we need to add code for the writer to this PR but please raise an issue.

A number of CI runners had issues at the installation step so I restarted them.

[orbeckst]

LGTM — thank you @schlaicha ! Congratulations to your first contribution to MDA 🎉

And many thanks for the quick review, @edisj , much appreciated!