search

openforcefield / openff-benchmark

Comparison benchmarks between public force fields and Open Force Field Initiative force fields
MIT License
11 stars 2 forks source link

Update coverage report #49

Closed jthorton closed 3 years ago

jthorton commented 3 years ago

Description

Fixes #46 and fixes #31 , the coverage reporter will now only test on one conformer per unique molecule. If the molecule passes all conformers are moved to the output folder if the molecule fails all conformers are moved the error_mols folder in the output. The coverage reporter now also records the total number of molecules screened along with the number of molecules that pass so we can work out a failure rate.

Todos

Notable points that this PR has either accomplished or will accomplish.

Questions

Status

codecov-io commented 3 years ago

Codecov Report

Merging #49 (1b87f3e) into master (896aad0) will decrease coverage by 1.23%. The diff coverage is 82.76%.

jthorton commented 3 years ago

I think this is now good to go. The testing failure seems to be due to PR #45 where the rmsd between conformers was changed, I can change the tests if you want @j-wags?

j-wags commented 3 years ago

I've just fixed the failing tests in a commit directly to master, so those will go away after the merge. Apologies for the delay. I'll give this a review before Monday's release, but I may not have time today. I'll merge it when ready. Thanks for all the hard work on this, @jthorton!

j-wags commented 3 years ago

Do we want to include the error messages in the coverage report? Can we do this without exposing sensitive data?

No and no

I've just taken a pass through this and made some small changes. Will let tests run and then will merge. This was a TON of high-quality new code and tests. Thanks a million, @jthorton!