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

Replace entire link instead of potential by potential #584

Open
Lp0lp opened this issue Apr 12, 2024 · 3 comments · May be fixed by #595
Open

Replace entire link instead of potential by potential #584

Lp0lp opened this issue Apr 12, 2024 · 3 comments · May be fixed by #595

Comments

@Lp0lp
Copy link
Collaborator

Lp0lp commented Apr 12, 2024

In the case where you have a default link which is made up of a sum of several potentials, i.e.:

[ link ]
resname $protein_resnames
[ dihedrals ]
-BB BB +BB ++BB 9 0 10 1
-BB BB +BB ++BB 9 20 10 1 {"version": 1}

And you want to replace it in a specific case (particular aminoacid for example) by a link made up of a single potential. i.e.:

[ link ]
resname $protein_resnames
[ dihedrals ]
-BB BB +BB ++BB 1 90 70 1

You are forced to introduce a second potential with fc=0 in order to override the second part of the default link. i.e.:

[ link ]
resname $protein_resnames
[ dihedrals ]
-BB BB +BB ++BB 9 90 70 1
-BB BB +BB ++BB 9  0  0 1

Otherwise, you end up with a mix of the default and the override. Would be cool (and help clean up .ff files) if we could somehow replace the entire link instead. Currently, we are ending up with dirtier .ff and .itp files full of "zeroed" potentials.

@pckroon
Copy link
Member

pckroon commented Apr 15, 2024

This is definitely an unfortunate side effect, thanks for pointing it out.
My suggestion would be the following: if an interaction does not specify a "version" it will replace all, and if it does specify a "version" it only affects that one.
I'll try to find some time soon-ish to implement this, but I can't make any promises.

As an aside/work-around, links can also /remove/ interactions, which should at least clean up your itps.

@fgrunewald
Copy link
Member

@pckroon I agree that this convention is the most sensible and easy(ish) to implement.

Conceptually, however, it would make a lot of sense to allow grouping of interactions (i.e. an interaction group - not to be confused with grouping them in the output). At the end of the day this type of dihedral potentials represent one interaction in the MD input as well. Changing just one term is a highly unlikely scenario and rather one wants to change all of them. GROMACS used to strictly enforce this by having them specified as interaction type 9, but that somehow got lost and now 1 and 9 are essentially the same.

In my view, if we were to completely abolish versions and just treat those interactions as a group, it would simplify a lot of the conceptual aspects of links. The code will likely be more complicated, which is why I am not pushing this right now.

@pckroon
Copy link
Member

pckroon commented Apr 24, 2024

That would indeed be the correct solution (I think), but does indeed cost a bit more code. I think I'll have to bite the bullet though. My previous suggestion has too many edgecases where what you want/need to do is either not possible or super hard. For example, when you want to switch from a "versionless" dihedral with a single potential to a "versioned" dihedral with multiple potentials.
The downside is that the parsing of interactions (not just for links!) gets harder, since it will no longer be line-by-line. I will cut myself some slack, and state that interaction groups must at least be within the same section, which should keep it manageable.

pckroon added a commit that referenced this issue May 6, 2024
@pckroon pckroon linked a pull request May 6, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants