mbuild
mbuild copied to clipboard
Replace decanexyz
PR Summary:
In testutils.py, test_structure_reproducibiliy would always xfail due to the decane.xyz standard file against which it would compare the newly-generated decane being outdated, as it was created with MDTraj instead of mbuild. This update now uses a standard file generated with the mbuild xyz writer instead. Due to the filename difference, the diff test was still failing, so the temp file was renamed to match the standard decane.xyz
PR Checklist
- [x] Includes appropriate unit test(s)
- [ ] Appropriate docstring(s) are added/updated
- [x] Code is (approximately) PEP8 compliant
- [ ] Issue(s) raised/addressed?
@rwsmith7531 and I worked together on this. The tests are passing on our machines 😂 . Im wondering if it is somehow using the old decane.xyz file still....
If someone can grab this branch and run the tests, im curious if this is somehow related to AZP.
This was a headache when I last touched this code, and that is why I marked this test flaky: https://github.com/mosdef-hub/mbuild/pull/658#issuecomment-565052401
However, you'd still want to to use decane.xyz
. The instructions for this test are
- Load a decane into memory
- Save it to
'decane.xyz'
- Compare the contents of
'decane.xyz'
and'decane-tmp.xyz'
If these file names are clobbered, the tests should pass automatically, since the comparison would presumably be against the same file. I'll also add that I should have named 'decane-tmp.xyz'
something like 'decane-ref.xyz'
Also, for what it's worth, this test passes locally. I have yet to open the AZP logs but there may be a separate issue there.
https://dev.azure.com/mosdef/mosdef/_build/results?buildId=904&view=logs&j=4bf9966a-0fce-5a84-b727-895eba4ceb25&t=66f48dc0-c1a0-5fef-b591-5ee2094f41e1
Just looking at the diffs in the two files one of them has -0.000000
and the other has 0.000000
, which could be cause by floating point error somewhere (?)
E AssertionError: assert not ['- C -0.000000 -11.200000 -0.000000\n', '+ C -0.000000 -11.200000 0.000000\n', '- H -1.100000 -11.20000...1.200000 0.000000\n', '- C -0.000000 -9.800000 -0.000000\n', '+ C -0.000000 -9.800000 0.000000\n', ...]
I also wonder if it would be easier to use a simple molecule like water or methane. The diff on the positions of however many atoms in decane produces an error that's hard to read
This is sort of relevant and kind of interesting https://stackoverflow.com/questions/4083401/negative-zero-in-python
Codecov Report
Merging #711 (1aa0776) into master (025abd4) will not change coverage. The diff coverage is
n/a
.
@@ Coverage Diff @@
## master #711 +/- ##
=======================================
Coverage 89.74% 89.74%
=======================================
Files 63 63
Lines 8178 8178
=======================================
Hits 7339 7339
Misses 839 839
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 025abd4...1aa0776. Read the comment docs.
In terms of getting this PR merged (2 years in the future), I think the simplest step is to follow @mattwthompson and just use a simpler molecule. @uppittu11 or @rwsmith7531 is this something you think you could give a try?