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

Opt out of solute centering in solvate #974

Closed
mphoward opened this issue Oct 29, 2021 · 8 comments
Closed

Opt out of solute centering in solvate #974

mphoward opened this issue Oct 29, 2021 · 8 comments

Comments

@mphoward
Copy link

Describe the behavior you would like added to mBuild

It would be nice to be able to opt out of centering the solute in mbuild.packing.solvate. If the solute is large, translating the center of mass may take it outside the box, leading to issues with packmol.

Describe the solution you'd like

Add an option like center=True to mbuild.packing.solvate. Then, update this template string

mbuild/mbuild/packing.py

Lines 30 to 36 in 0eca09d

PACKMOL_SOLUTE = """
structure {0}
number 1
center
fixed {1:.3f} {2:.3f} {3:.3f} 0. 0. 0.
end structure
"""

so that it only uses the center option & nonzero fixed when center=True. Otherwise, only use fixed 0 0 0 0 0 0.

Describe alternatives you've considered
I've tried workarounds for this, but I couldn't find something that keeps the solute from translating.

@CalCraven
Copy link
Contributor

Did this ever get fixed? Seems like a simple enough fix in the arguments to mbuild.packing.solvate! Marking this as a good first issue.

@mphoward
Copy link
Author

I am happy to contribute this if the proposed fix above is sufficient. Or, do you want to wrap this into PR #787 that you linked?

@daico007
Copy link
Member

Hi @mphoward, it would be great if you can open a PR with the solution posted above!

@mphoward
Copy link
Author

Thanks, will do, probably in 1–2 weeks! (We are starting a new semester here on Tuesday.) The timing on this is excellent because I am just about ready to really want to use this feature in production. =)

@CalCraven
Copy link
Contributor

@mphoward Yeah that would be great. I think conversation has started back up on the PR mentioned above, which should be a more holistic overhaul of the PackMol functionality in mBuild. @chrisjonesBSU seems to have a few different things that he would like to wrap in all at once. I would suggest looking at the API that the PR is proposing to add using a packmol_args argument as a dictionary, and implementing the resulting affect to the packmol file that's used to generate the packing.

The two questions I have are:

  1. What would be the argument that would be intuitive here? {"center_solute":False} makes sense to me I think, as you mentioned above.
  2. Which arguments are valid for multiple different packing methods, and which are specific to each (fill_box, solvate, fill_sphere etc.)? This seems specific to just the solvate function.

@mphoward
Copy link
Author

2. Which arguments are valid for multiple different packing methods, and which are specific to each (fill_box, solvate, fill_sphere etc.)? This seems specific to just the solvate function.

This was focused on just the solvate function. I am not sure if center makes sense (or indeed is used) in those other methods. In that sense, I am not sure whether or not this proposed change exactly fits in with the design of the other API (and welcome comments on it), as center only applies to the solute and not the other molecules in packmol. In that sense, it might be cleaner to retain only a center argument for the function itself, then accept additional packmol_args that apply to everything as in the other PR?

@chrisjonesBSU
Copy link
Contributor

I agree that in the interest of usability that we keep some of the more applicable and useful packmol options as their own parameter in the individual packing functions. Such as center_solute=True in the solvate function and edge in all of the packing functions. I know this contradicts one of the comments made in #787, but that discussion is nearly 2 years old.

This makes the useful packmol options more accessible rather than expecting the user to read through and understand all of the possible packmol parameters.

@daico007
Copy link
Member

Implemented in #1144

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants