Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update apply_forcefield and molecule forcefield #62

Merged
merged 41 commits into from
Sep 29, 2023
Merged

Update apply_forcefield and molecule forcefield #62

merged 41 commits into from
Sep 29, 2023

Conversation

marjanalbooyeh
Copy link
Collaborator

@marjanalbooyeh marjanalbooyeh commented Sep 25, 2023

This PR applies following changes to the Molecule and System classes:

  1. Move force_field and auto_scale parameters from System init to apply_forcefield method.
    Since applying forcefield is now done after system initialization, it makes sense to move all forcefield related parameters to apply_forcefield.
    The apply_forcefield method always sets the reference_values dictionary. When auto_scale=True the reference values will be set based on the maximum parameter value and when its false, the values are set to 1 with default gmso units. As a result, we should always use reference_values as the base_units parameter when calling gmso hoomd functions.

  2. Added base classes for forcefield
    Two new classes are added to the base module: BaseXMLForcefield and BaseHOOMDForcefield. All the forcefield classes defined in the library now inherit from one of these two base classes. The base classes do not have any additional functionalities, they are defined to ensure all subclasses have similar attribute names. This consistency in attribute names is needed in forcefield validations done in System and Molecule classes.

  3. Allow forcefield to be specified in Molecule class
    We can pass a valid forcefield to the Molecule class when initializing the molecules. In this case, apply_forcefield can be called without passing the forcefields and the system class uses the forcefields from the molecules.
    Note: Previously, the Molecule class had the forcefield parameter but the accepted types were either direct xml files or list of hoomd objects. With the changes in this PR, only valid forcefields, i.e. subclasses of the base forcefield classes, can be passed to the Molecule.

@codecov
Copy link

codecov bot commented Sep 26, 2023

Codecov Report

Merging #62 (a0da905) into main (d71f21e) will increase coverage by 0.06%.
The diff coverage is 96.20%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #62      +/-   ##
==========================================
+ Coverage   93.63%   93.70%   +0.06%     
==========================================
  Files          18       19       +1     
  Lines        1383     1397      +14     
==========================================
+ Hits         1295     1309      +14     
  Misses         88       88              
Files Coverage Δ
hoomd_organics/base/__init__.py 100.00% <100.00%> (ø)
hoomd_organics/base/forcefield.py 100.00% <100.00%> (ø)
hoomd_organics/base/molecule.py 96.55% <100.00%> (-0.08%) ⬇️
hoomd_organics/library/__init__.py 100.00% <ø> (ø)
hoomd_organics/library/forcefields.py 100.00% <100.00%> (ø)
hoomd_organics/utils/__init__.py 100.00% <100.00%> (ø)
hoomd_organics/utils/base_types.py 100.00% <ø> (ø)
hoomd_organics/utils/ff_utils.py 89.58% <100.00%> (-2.34%) ⬇️
hoomd_organics/base/system.py 90.61% <92.50%> (+1.11%) ⬆️

... and 1 file with indirect coverage changes

@marjanalbooyeh marjanalbooyeh marked this pull request as draft September 26, 2023 19:14
@marjanalbooyeh marjanalbooyeh changed the title Update auto scale interface Update apply_forcefield and molecule forcefield Sep 28, 2023
@marjanalbooyeh marjanalbooyeh marked this pull request as ready for review September 28, 2023 21:52
Copy link
Member

@chrisjonesBSU chrisjonesBSU left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thanks!! I just left one comment about a doc string.

hoomd_organics/base/molecule.py Outdated Show resolved Hide resolved
@marjanalbooyeh marjanalbooyeh merged commit f470832 into cmelab:main Sep 29, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants