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

[BUG] Unable to parameterise Chiral center with charge ligand by using OFF200 #370

Closed
Roy-Haolin-Du opened this issue Nov 20, 2024 · 9 comments · Fixed by OpenBioSim/sire#263
Labels
bug Something isn't working

Comments

@Roy-Haolin-Du
Copy link

Roy-Haolin-Du commented Nov 20, 2024

Describe the bug

I am unable to parameterize this ligand with OFF2.0.0 using BioSimSpace, starting from an sdf
It has three chiral centers and +1 charge.
When I used biosimspace and sire == 2024.3.0 or 2024.4.0.dev, the error is 'Unable to create OpenFF Molecule!', what is wrose, this version can not parameterise Chiral molecules without charge having Same error.

When I used biosimspace and sire == 2024.2.0 or less, the error is 'Unable to create OpenFF Interchange object!',but it can parameterise Chiral molecules without charge.

image

To Reproduce
In [1]: import BioSimSpace as BSS
In [2]: lig = BSS.IO.readMolecules("lig_C48.sdf")[0]
In [3]: lig_param = BSS.Parameters.openff_unconstrained_2_0_0(lig).getMolecule()
╭─────────────────────────────── Traceback (most recent call last) ────────────────────────────────╮
│ in :1 │
│ │
│ /home/roy/mambaforge/envs/openbiosim_3.10/lib/python3.10/site-packages/BioSimSpace/Parameters/_p │
│ rocess.py:244 in getMolecule │
│ │
│ 241 │ │ │ │ │ "Parameterisation failed! Last error: '%s'" % str(self._last_error) │
│ 242 │ │ │ │ ) │
│ 243 │ │ │ else: │
│ ❱ 244 │ │ │ │ raise _ParameterisationError( │
│ 245 │ │ │ │ │ "Parameterisation failed! Last error: '%s'" % str(self._last_error) │
│ 246 │ │ │ │ ) from None │
│ 247 │
╰──────────────────────────────────────────────────────────────────────────────────────────────────╯
ParameterisationError: Parameterisation failed! Last error: 'Unable to create OpenFF Molecule!'

In [4]: lig = BSS.IO.readMolecules("lig_31.sdf")[0]

In [5]: lig_param = BSS.Parameters.openff_unconstrained_2_0_0(lig).getMolecule()
╭─────────────────────────────── Traceback (most recent call last) ────────────────────────────────╮
│ in :1 │
│ │
│ /home/roy/mambaforge/envs/bsstutorials/lib/python3.10/site-packages/BioSimSpace/Parameters/_proc │
│ ess.py:244 in getMolecule │
│ │
│ 241 │ │ │ │ │ "Parameterisation failed! Last error: '%s'" % str(self._last_error) │
│ 242 │ │ │ │ ) │
│ 243 │ │ │ else: │
│ ❱ 244 │ │ │ │ raise _ParameterisationError( │
│ 245 │ │ │ │ │ "Parameterisation failed! Last error: '%s'" % str(self._last_error) │
│ 246 │ │ │ │ ) from None │
│ 247 │
╰──────────────────────────────────────────────────────────────────────────────────────────────────╯
ParameterisationError: Parameterisation failed! Last error:[ 'Unable to create OpenFF Molecule!'].

But, It does work with allow_undefined_stereo=True, that causes issues further down the line.

lig_31.sdf is chiral no charge. lig_C48.sdf is chiral with +1 [[charge.]]
ligands.zip

Version of Python: 3.10
rdkit version is suitable for BSS version.
rdkit and librdkit 2024.03.5 for sire and bss ==2024.3.0 and 2024.4.0.dev
rdkit and librdkit 2024.03.3 for sire and bss == 2024.2.0

Many thanks for your help.

@Roy-Haolin-Du Roy-Haolin-Du added the bug Something isn't working label Nov 20, 2024
@lohedges
Copy link
Contributor

The more recent version of BioSimSpace uses the Sire-to-RDKit conversion to avoid going via an intermediate file. This must be the reason why you can't parameterise a chiral molecule without charge. I'll take a look at the resulting RDKit molecule (loading from SDF and direct conversion) to see what the difference is.

With BSS.setVerbose(True) you can see the full error message. For the chiral ligand with charge this is:

ParameterisationError: Parameterisation failed! Last error: 'Unable to create OpenFF Molecule!: UndefinedStereochemistryError('Unable to make
OFFMol from RDMol: RDMol has unspecified stereochemistry. RDMol name: MOLUndefined chiral centers are:\n - Atom C (index 3)\n - Atom C (index
6)\n - Atom C (index 7)\n - Atom C (index 23)\n')'

As you say, it will work with allow_undefined_stereo=True, but this isn't likely to work in practice due to knock-on effects. I'll check the SDF to see if there's anything obvious that's wrong.

@lohedges
Copy link
Contributor

Hello there,

I've had reports of similar issues for ligands that worked with the previous release. I am tracking the debugging here. The issue appears to be with the Sire-to-RDKit conversion for certain molecules. Annoyingly the directly conversion fixed other issues, so it's hard to just revert the change, i.e. something would be broken with either approach. I also can't try both methods, since the conversion sometimes works but gives invalid output, so you could end up silently creating a molecule that is incorrect.

I'll update the other thread as I make progress.

Cheers.

@Roy-Haolin-Du
Copy link
Author

I sincerely appreciate your assistance in addressing such a trivial issue. I had update the latest version of BSS and Sire. And try chiral ligands with charge and without charge. All of them work, and are parameterised well. I am going to test more highthrought running to see how robust these tools are to help computation.

@lohedges
Copy link
Contributor

lohedges commented Dec 6, 2024

Many thanks for the update, it's great to hear that things are robust so far for the ligands that you have tested. Please do let me know if you come across further issues. I have now have a much better understanding for how stereochemistry is applied on conversion between Sire and RDKit format.

@lohedges
Copy link
Contributor

Hi there, just to make aware of this recently discovered bug in the SIre SDF that meant that certain positive formal charge values were parsed incorrectly. While I had fixed the writing of these records, I hadn't noticed that the reading was wrong. I've checked, and this was the ultimate source of the issue that you posted here, i.e. the charge was set as +3 rather than +1. As such, you might want to re-parameterise your ligands (using the development conda package once the PR is merged) to check whether anything has changed.

@Roy-Haolin-Du
Copy link
Author

Thanks so much for letting me know, I will re-run these charged ligands once the PR is merged and keep following this update.

@lohedges
Copy link
Contributor

This has now been merged and is available from the devel channel, or in the 2024.4.1 patch release.

@Roy-Haolin-Du
Copy link
Author

Hi Lester, I just test two charged ligands, one is -2 which has totally same parameters. Another is +1, which has minor difference for charge shown below. Seems no effect on these charged ligands. Thanks so much!

Image

@lohedges
Copy link
Contributor

Thanks for confirming. Pleased to hear that all is okay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants