Evaluating coverage

[BFedder]

Currently, Codecov shows 82 % coverage for panedr. 47 out of 261 lines are missed by the tests.

Of those 47, 30 are missed because only the most recent EDR file format (5) is currently tested. This file format has not changed in a while. 11 years ago, at Gromacs version 3.2.0, the EDR file format was already at 5:

Excerpt from the file linked above:

/* This number should be increased whenever the file format changes! */
static const int enx_version = 5;

From panedr:

ENX_VERSION = 5

The source code for versions of Gromacs preceding 4.5 is no longer available on their website, so I personally think not testing against the older file formats is safe. Thus, 30 of the 47 misses are fine, and really, coverage is closer to 93 % than to 82 %.

A number of the remaining 17 misses are due to the fact that no corrupted EDR files are part of the test set. Another missed line (111) can be covered by trying to open a non-EDR binary file.

I will go over the misses in more details still, but at first glance, they do not seem problematic to me.

[orbeckst]

Note that PR #22 increases coverage to 87% (together with some legacy format support).

[orbeckst]

Anything that we do not want to test, we could mark up with # pragma: no cover. That will make clear in the code which lines are known to be not tested.

There are different schools of thought if it's better to show a lower coverage or to add the no-cover. What is the opinion of @jbarnoud and @IAlibay on the matter?

[IAlibay]

Looks to me like the code doesn't allow anything else than the latest version. In that case the extra code paths are likely dead and should be removed? We could mock a few things to trigger the errors too.

[jbarnoud]

The code is a translation of the gromacs code. It is meant to cover everything gromacs can throw at an edr file. I don't know if gromacs has tests for their edr reader, but that may be a good place to start looking for challenging inputs.

[BFedder]

Reading old EDR file versions should be allowed in the code.

• at line 101, an integer is unpacked. This should be -55555 in all energy files but version 1
• At line 113, a second integer is unpacked. This integer is the file version number
• The check at 114 compares the file version with the version of gmx energy / panedr, file versions newer than 5 raise an exception
• The check at line 117 warns the user when reading old file formats, but allows execution

[jbarnoud]

Yes, the code should deal with multiple version of the EDR format. #22 should allow going even older. Reading old files is important for reproducibility: I've seen many cases where somebody wanted to check properties from an ancient simulation using a modern script. But it also means we need to generate old files to test some code paths.

[BFedder]

I have checked a few sources of regression tests for various GROMACS versions, but I have only been able to find EDR files of version 1 or version 5. Versions 2, 3, and 4 are absent from even the oldest version of the regression tests I could find (GROMACS 4.6, found at https://ftp.gromacs.org/pub/regressiontests/). #22 would as far as I can tell patch in support and tests for version 1 of the file format.

I am not sure when those missing versions where actually in use, because even at GROMACS 3.2, EDR file format 5 was already in use. Do any of you perhaps have the source code for GROMACS 2 lying around, or perhaps an energy file written by that version? I can't find that anywhere, and that would be my best bet at adding further tests. This obviously isn't the highest priority here, though.

[orbeckst]

Testing old file formats is worthwhile but it's not the focus of @BFedder 's project. I would say, right now testing and coverage here is "good enough" and just reviewing and merging PR #22 would get us a long way covering other parts of the code.

Therefore, I suggest @BFedder focuses on #34 and the PR #33 as this is moving his project forward.

[ezavod]

Hi all, I'm the author od #22 and still alive :-)

I am not sure when those missing versions where actually in use, because even at GROMACS 3.2, EDR file format 5 was already in use.

AFAIK file versions 2, 3, 4 were never actually released. Version 4.0.7 (06.12.2009) still uses file version 1 (git checkout v4.0.7; ag -Qi enx_version . finds no hits in the gromacs github). The next release 4.5 (01.09.2010) uses already file version 5. Between those two releases there are no others tagged.

Testing old file formats is worthwhile but it's not the focus of @BFedder 's project.

I'm happy to help with integration of #22 (if wanted). In case there is anything to discuss, feel free to ping me.

---

@BFedder I just read about the project on your blog. Good luck and have fun!

[BFedder]

AFAIK file versions 2, 3, 4 were never actually released.

Ah that's interesting, weird, and useful, thanks!

Good luck and have fun!

Thank you :)

[orbeckst]

@BFedder , how about reviving PR #22 ?

[IAlibay]

Coverage is now at 100%