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

MSIBI Revamp #39

Open
wants to merge 620 commits into
base: main
Choose a base branch
from
Open

MSIBI Revamp #39

wants to merge 620 commits into from

Conversation

chrisjonesBSU
Copy link

@chrisjonesBSU chrisjonesBSU commented Aug 22, 2024

Part of my work over the last few years involved using the MISBI method to create CG potentials for polymers,
and a lot of effort was spent by myself and @jennyfothergill to build up a working python package for doing that, building on top of what was already in this repo. This resulted in essentially a complete revamp. After talking with my PI (Eric Jankowski) we felt it was time to make a PR to mosdef-hub for better visibility and engagement. This isn't a finished product, but is much improved and so far seems to work well for learning bond, angle and pair potentials. The README.md file has a couple examples of the new API.

Main additions include:

  • Learning more than just pair potentials. IBI is especially useful for learning angle potentials for polymers.
  • Setting multiple forces in the MSIBI iterations (e.g. Setting fixed bond and angle potentials while learning pairs)
  • Uses Hoomd 4 and Freud
  • Convenient methods for saving forces, table potentials, plots, etc..
  • Target distributions are calculated for you when creating State and Force objects
  • Improved testing coverage
  • Improved documentation

Main To Dos remaining are:

  • Make a set of tutorials with the new API
  • Add helpful converters so that other trajectory file types can be passed in
  • Further testing and validation

chrisjonesBSU and others added 30 commits April 4, 2022 12:33
Add functionality to perofrm IBI on bonded parameters
Normalize the target bond and angle distributions
Add nlist, exclusions, and smoothing of final potentials
Copy link
Member

@ctk3b ctk3b left a comment

Choose a reason for hiding this comment

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

Excited to see this! I probably won't have time to fully review but one small suggestion: can we break this up into smaller distinct PRs? I see at least 3 that would be much easier to review:

  • Changes to CI/test/lint infra
  • Doc updates
  • The core of your changes (I suspect these could also be teased apart but hard to recommend how without diving a lot deeper)

@chrisjonesBSU
Copy link
Author

@ctk3b I'm glad you were notified by the PR! Maybe @tcmoore3 will stop by too, although I've talked to him a few times about all these changes when we run into each other at conferences. I totally understand if you don't have time to do a thorough review, though.

In regard to splitting this PR into multiple PRs, while I totally agree that would be a more sensible approach, I'd still prefer to do it all in this PR. Mostly for the sake of time. But, also because the goal of this PR is not necessarily to be a final version ready for release, but to move further development and maintenance of MSIBI to the MoSDeF repository instead of our lab's fork. Now that I'm an owner/admin with MoSDeF I'll continue closing up any loose ends over here through the more sensible use of smaller issues and PRs. I know this isn't ideal, but I think getting all of these changes upstream will be beneficial.

Most of these changes were done over several separate PRs in our lab's fork of MSIBI which you can see here. Examples of the API can be seen in our fork's README.

In regard to the CI changes, they are essentially an exact copy of what we're now doing in mBuild, Foyer, and GMSO. So, that is to say, I think the CI changes here are already pretty well reviewed and vetted from when we implemented them in these other packages, and for consistency's sake, the plan is to use it here as well.

This PR shows the majority of the documentation changes. It's mostly added sections for the new classes (Force, Pair, Bond, Angle, Dihedral).

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.

3 participants