Becksteinlab / MDPOW

Calculation of water/solvent partition coefficients with Gromacs.
https://mdpow.readthedocs.io
GNU General Public License v3.0
25 stars 10 forks source link

initialize guess_atoms function PR #264

Closed cadeduckworth closed 1 year ago

cadeduckworth commented 1 year ago
codecov[bot] commented 1 year ago

Codecov Report

Merging #264 (3f90ad2) into develop (10922e8) will increase coverage by 0.17%. The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop     #264      +/-   ##
===========================================
+ Coverage    81.12%   81.29%   +0.17%     
===========================================
  Files           15       15              
  Lines         1960     1978      +18     
  Branches       296      297       +1     
===========================================
+ Hits          1590     1608      +18     
  Misses         278      278              
  Partials        92       92              
Files Changed Coverage Δ
mdpow/workflows/base.py 83.07% <100.00%> (+5.52%) :arrow_up:
mdpow/workflows/dihedrals.py 95.97% <100.00%> (+0.04%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

cadeduckworth commented 1 year ago

@orbeckst please review

I did not try it out on any cases outside of the notebook during development, however, I did incorporate usage into the dihedrals module and also added the test for the base module. Everything passed locally, so hopefully there are no issues with the CI tests.

If you see any issues please let me know. If everything looks good I will update my SAMPL9 environment, do a test run, and add documentation once we are sure everything is working as intended.

If you would like more detail on the new loop added for creating the boolean array used for identifying problem cases I can upload the updated notebook here.. just let me know!

orbeckst commented 1 year ago

This needs some debugging. Ideally you can put whatever kills it at the moment in a test case.

orbeckst commented 1 year ago

@cadeduckworth see the bug https://github.com/MDAnalysis/mdanalysis/issues/4167 for LigParGen atom types and the fix PR https://github.com/MDAnalysis/mdanalysis/pull/4168 which is now in the latest released version of MDA. Do we still need our own fix?