Expand water virtual site tests #680
Conversation
|
Hello @IAlibay! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2024-01-15 23:19:32 UTC |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #680 +/- ##
==========================================
+ Coverage 91.40% 91.43% +0.02%
==========================================
Files 132 132
Lines 9109 9140 +31
==========================================
+ Hits 8326 8357 +31
Misses 783 783
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
@hannahbaumann this is another good one to review if you want to get familiar with a specific section of the HTF. |
| vs_weights, [0.786646558, 0.106676721, 0.106676721] | ||
| ) | ||
| c, s, e = nonbond.getParticleParameters(entry) | ||
| assert ensure_quantity(c, 'openff').m == pytest.approx(-1.04844) |
There was a problem hiding this comment.
should we look at contributing some sort of assert_quantity_equal(val, expected_mag, expected_unit) to openff units?
There was a problem hiding this comment.
If Matt wants to? We could also just make our own tooling if it's necessary.
| @@ -458,7 +461,58 @@ def test_dry_run_ligand_tip4p(benzene_system, toluene_system, | |||
| with tmpdir.as_cwd(): | |||
| sampler = dag_unit.run(dry=True)['debug']['sampler'] | |||
| assert isinstance(sampler, MultiStateSampler) | |||
| assert sampler._factory.hybrid_system | |||
|
|
|||
| # Get the htf and check | |||
There was a problem hiding this comment.
is it going to be a pain to make this a fixture and make these three tests individual?
There was a problem hiding this comment.
Would have to try, but iirc tmpdir is function scoped, so you can't use it in a module scoped fixture - at least I remember this being a thing in the past. (definitely wouldn't recommend this being function scoped, it's super slow)
There was a problem hiding this comment.
ah yeah, there's tmp_path_factory which has longer scope
Adds more checks #678
TODO:
Developers certificate of origin