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

"forcefield" property #147

Open
SofiaBariami opened this issue Mar 9, 2020 · 10 comments
Open

"forcefield" property #147

SofiaBariami opened this issue Mar 9, 2020 · 10 comments

Comments

@SofiaBariami
Copy link
Collaborator

Hello, I am not sure whether I should post this question here or at the BioSimSpace repository, but I am trying to understand how we can set the "forcefield" property that is required when we want to merge two molecules. The standard process to set properties is to create an editable molecule and atoms and then set all the properties separately, as we can see in amber.cpp, but I can't find anything on forcefield.
At the BSS code, we can see at _molecule.py that this property is required and it should also have a specific Amber format.
Does anyone have any thoughts about that?
Thanks a lot!

@lohedges lohedges transferred this issue from michellab/Sire Mar 9, 2020
@lohedges
Copy link
Member

lohedges commented Mar 9, 2020

Hi @SofiaBariami, I agree that is kind of a dual Sire/BioimSpace question but I've transferred this issue over here since the issue (for now) is likely specific to the creation of merged molecules in BioSimSpace.

The forcefield property of a Sire molecule is an MMDetail object. For an AMBER-style system it might look something like this. (Here I'm querying the underlying Sire object of a molecule in BioSimSpace that was loaded from AMBER format files.)

molecule._sire_object.property("forcefield")
MM ForceField{ amber::ff,
               combining_rules = arithmetic,
               1-4 scaling = 0.833333, 0.5,
               nonbonded = coulomb, lj,
               bond = harmonic, angle = harmonic,
               dihedral = cosine }

You can see how this property is set in the new AmberPrm parser here, and similarly for the GroTop parser here. It looks like this property is not used or set in the old amber.cpp parser that you refer to.

In BioSimSpace, we currently only support merges between two molecules that have compatible AMBER-style forcefields. The MMDetail object provides a convenient way of checking for compatibility and could be extended to support additional forcefield types in future.

The following code snippet shows how the MMDetail object is used within the private _merge method of the BioSimSpace Molecule class to check whether it is possible to merge two molecules based on their forcefield property.

# Get the user name for the "forcefield" property.
ff0 = inv_property_map0.get("forcefield", "forcefield")
ff1 = inv_property_map1.get("forcefield", "forcefield")

# Force field information is missing.
if not molecule0.hasProperty(ff0):
    raise _IncompatibleError("Cannot determine 'forcefield' of 'molecule0'!")
if not molecule1.hasProperty(ff1):
    raise _IncompatibleError("Cannot determine 'forcefield' of 'molecule1'!")
# The force fields are incompatible.
if not molecule0.property(ff0).isCompatibleWith(molecule1.property(ff1)):
    raise _IncompatibleError("Cannot merge molecules with incompatible force fields!")

I hope this makes sense. I'm happy to discuss more if you need a clearer explanation. If you think that you might need to extend the MMDetail class to support any additional forcefields that you are implementing then I should be able to offer some guidance.

@SofiaBariami
Copy link
Collaborator Author

Hi Lester, thank you very much. This is very helpful. I am indeed trying to use BioSimSpace to work with molecules that are parameterised with another forcefield, and since the forcefield property was not set, python was complaining. Maybe extending the MMDetail class is the best way to proceed with this, especially if we are planning to use QuBe in the future as well. I have managed to overcome some of the errors in hacky ways. For example I substituted ff.electrostatic14ScaleFactor() and ff.vdw14ScaleFactor() of the _merge function with the actual values of the scale factors, but this practice is not the best, since the code is complaining in other steps of the process as well. For example when I am calling _toPertFile() I am getting an Exception 'SireBase::missing_property' because of the same thing:

--> 709   if not self._sire_object.property("forcefield0").isAmberStyle():
    710       raise _IncompatibleError("Can only write perturbation files for AMBER style force fields.")

@jmichel80 what do you think? Should we extend MMDetail, or it is indeed too much of a detail, so find another way to overcome the issue?

@jmichel80
Copy link
Contributor

jmichel80 commented Mar 9, 2020 via email

@lohedges
Copy link
Member

Much as I'd like to take credit for it, the updated AMBER parser is all @chryswoods work :-)

@SofiaBariami
Copy link
Collaborator Author

you can find the files and the script here: https://github.com/SofiaBariami/qube_project/tree/master/merge_molecules

@jmichel80 when you say to post code for the Amber ff case, do you mean to refer to the BSS code that we use? If so, we are using the following main functions:

@jmichel80
Copy link
Contributor

jmichel80 commented Mar 10, 2020 via email

@SofiaBariami
Copy link
Collaborator Author

Thanks for the clarifications Julien.

  1. When it comes to Amber, two sets of prm7/rst7 files are read with readMolecules(), to form two BioSimSpace._SireWrappers._system.System. From these system, we can extract the molecules that are of type BioSimSpace._SireWrappers._molecule.Molecule. Next we need to use matchAtoms() that finds mappings between atom indices in molecule 0 to those in molecule 1. The input of this function must be of type Molecule <BioSimSpace._SireWrappers.Molecule>. The first problem here is that when reading pdb/xml files, the output molecule will be a Sire.Mol._Mol.Molecule. However, with this is solved easily with BSS._SireWrappers.Molecule(), that turns a Sire molecule into a BSS molecule. This was the main problem that made me change the functions, but with this fix, we can use the BSS functions as they are.
  2. The second problem is the one with that we discussed with Lester above. During the merge of two molecules, BSS checks whether their forcefields are compatible. When we are using Amber files, the forcefield property is set, and looks like this:
In [20]: mol1._sire_object.property("forcefield")                                                           
Out[20]: 
MM ForceField{ amber::ff,
               combining_rules = arithmetic,
               1-4 scaling = 0.833333, 0.5,
               nonbonded = coulomb, lj,
               bond = harmonic, angle = harmonic,
               dihedral = cosine }

However, the molecule loaded from xml/pdb, does not have this property at all, that results to an Exception 'SireBase::missing_property'. In order to solve this problem we should modify mmdetail and amberprm.cpp to accept input from xml/pdb with the qube forcefield. Since there are separate files for the file formats in Sire.IO (grotop, amberprm) I am not sure whether we should make new files for the xml/pdb files.
The easy, temporary fix that I did, in order to proceed with my project quickly, is that I just commended out the lines where the forcefield property was checked. I know that this doesn't solve the issue, but I decided to ignore this bit until we come up with a solution, without letting it interfere with the progress of the project.

@lohedges
Copy link
Member

I'm not completely sure how you are currently constructing a Sire/BioSimSpace system parameterised with the QuBe forcefield, but this is what I think would be the ideal solution...

Hopefully you are using the Sire PDB parser, then adding properties to the System object with the information that you parse from the XML file. (Although, perhaps you are doing this the other way around.) This is how the Sire parsers are intended to work: one parser constructs the initial system (using the molecular topology), then other parsers can add additional information to the existing System. You can read more details about this in the BioSimSpace paper here.

Ideally there would be a new Sire parser called QubeXML (or similar) that could parse the forcefield information from XML file. This would create its own MMDetail object which would be added as a property of each molecule in the System. If the forcefield can have terms that we don't currently support then we'd also need to update MMDetail to reflect this.

Obviously all of the above is quite involved and requires a lot of knowledge of Sire.

@lohedges
Copy link
Member

lohedges commented Mar 11, 2020

Hi @SofiaBariami, I noticed that you recently pushed to the feature-vSites branch. Is the feature_virtual-sites branch now redundant? If so, I'll delete it since I'd like to avoid a proliferation of stale branches.

Cheers.

@SofiaBariami
Copy link
Collaborator Author

hi Lester, yes, can you delete the feature_virtual-sites branch please? I left it to delete = it myself later on, but I don't need it anymore

annamherz pushed a commit that referenced this issue Mar 5, 2024
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

No branches or pull requests

3 participants