change default of use_residue_map to be False

mosdef-hub / foyer

A package for atom-typing as well as applying and disseminating forcefields

https://foyer.mosdef.org

MIT License

117 stars 75 forks source link

change default of use_residue_map to be False #535

Closed daico007 closed 1 year ago

daico007 commented 1 year ago

PR Summary:

Address https://github.com/mosdef-hub/mbuild/issues/1111. Change the default of use_residue_map in Forcefield.apply to False, since this may be produced unexpected behavior if users try to atomtype of systems with similar residue name.

PR Checklist

[ ] Includes appropriate unit test(s)
[ ] Appropriate docstring(s) are added/updated
[ ] Code is (approximately) PEP8 compliant
[ ] Issue(s) raised/addressed?

daico007 commented 1 year ago

Tests failed due to weird issue when calling pmd.version (stating that parmed doesn't have version module). I still couldn't replicate this locally (update parmed to v4.1.0, and parmed.version is still returning a version object.

Update: turn out there is a typo

codecov[bot] commented 1 year ago

Codecov Report

Merging #535 (c140810) into main (14aa3d3) will increase coverage by 0.10%. The diff coverage is 100.00%.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #535 +/- ## ========================================== + Coverage 69.07% 69.18% +0.10% ========================================== Files 16 16 Lines 1675 1671 -4 ========================================== - Hits 1157 1156 -1 + Misses 518 515 -3 ```

daico007 commented 1 year ago

Update, the issue has drifted a bit, now the tests are failing because of the apply_NBFIX test, there is some behavior change due to new version of parmed (which affect the uniqueness of atom_type). Working on the fix atm