-
Notifications
You must be signed in to change notification settings - Fork 3
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
Address open questions on SMIRNOFF format spec revamp #48
Comments
It looks like, using OE tools, I can calculate Wiberg bond orders using https://docs.eyesopen.com/toolkits/python/quacpactk/bondordertheory.html . Is there an equivalent method we have in mind using the RDKit/AmberTools toolkits? |
@j-wags from what I looked into in the past, I don't think there is the equivalent in RDKit. |
@j-wags Right, there is nothing in AmberTools for this yet. You may have noticed this was also something which came up in the Torsions call and we reminded @dgasmith about the need for it. For now the RDKit implementation would presumably have to raise an exception if it is asked to do something with bond orders. (We don't yet have a force field which requires bond orders, so these are an optional -- but important for the future -- feature.) |
Commit 246101b moves remaining spec questions from Should we have a separate
|
Per openforcefield/openff-toolkit#233 (review) Should the SMIRNOFF spec live in its own repo, instead of being bundled with the toolkit documentation? |
Hmm, the spec in its own repo? That's an interesting idea. Pros and cons? |
Examples: |
Copying relevant text from openforcefield/openff-toolkit#341 (comment) Here are some unresolved questions we may need to solveWhat is the relationship between a parameter section and the SMIRNOFF tag?I actually don't know, so I'm going to "think out loud" here. The SMIRNOFF tag currently encodes the aromaticity model. Could we achieve the same behavior by having each parameter section have an aromaticity model? Yes, we could. So maybe, the idea of an enclosing tag in the SMIRNOFF spec indicates "a value that would otherwise need to be set repeatedly in the enclosed tags". In that sense, the top-level As a thought experiment, I could make a new ParameterHandler that applies parameter based on a different aromaticity model, and have it override the higher level aromaticity model. This is a weird example, but I couldn't come up with a better one -- The point is that it would seem to be arbitrary to tell a developer they can't do that, just because we say so. Note that the above "sections should be enclosed in other sections if the enclosing section affects the behavior of the enclosee" logic would have a consequence for @andrrizzi's question in openforcefield/openff-toolkit#310 (comment)
It implies that the sections that describe atomic charges should be enclosed by the Electrostatics tag (since attributes like the cutoff influence the functional form). It also opens the question of "what if a person wanted both the Electrostatics and GBSA tags? What should enclose the charge-determining sections?". I don't know what we'd do in that situation. And, in a similar vein, this will make things like VirtualSites more difficult, since they currently are in their own section, but will experience electrostatics and vdW forces. Should they be doubly-nested in Electrostatics and vdW tags? Or should the VirtualSite definitions be duplicated in the enclosing Electrostatics and vdW sections? So maybe this isn't the right view of section enclosure. Maybe the SMIRNOFF version encodes something about the underlying section structureFor example, in the 0.2 -> 0.3 transition, we switch from units-in-the-header to units-in-the-quantity. Well, I'd argue that even that change could have been handled at the parameter section level. So one could argue that changing to the "SMIRNOFF 0.3 spec" doesn't actually mean anything -- it's the parameter sections that need to change their behavior! But, I guess the SMIRNOFF spec change here records a "rule change", because the SMIRNOFF 0.3 spec allows ParameterHandlers with their own versions. Is a parameter section's version truly just the
|
This is no longer strictly in the domain of the toolkit so I'm transferring it to the |
https://open-forcefield-toolkit.readthedocs.io/en/topology/smirnoff.html lists a number of open questions raised by @jchodera (and relevant to @j-wags ) on the SMIRNOFF format spec. I'll give draft answers to some of them here:
metadata
orprovenance
section that users can add whatever they want to?" Seems like a good idea to me. Probably "metadata".<Bonds>
?" Yes! That is supposed to be implemented! We must have lost the issue for finishing that implementation when we migrated repos. @j-wags take note; we need to apply the same infrastructure that we use forBonds
for angles and torsions.id
are very useful though. Maybe we should have some optional tags (likeid
) which won't raise warnings/exceptions if present, but if anything other than required/optional tags is present it will raise an exception?The text was updated successfully, but these errors were encountered: