From bd340a6ca993161a9c2f5f5c7208106418db3e92 Mon Sep 17 00:00:00 2001 From: Josh Horton Date: Thu, 6 Jun 2024 16:49:36 +0100 Subject: [PATCH 1/5] make sure user charges are defined on atoms --- gufe/components/explicitmoleculecomponent.py | 9 +++++++++ gufe/tests/test_smallmoleculecomponent.py | 17 +++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/gufe/components/explicitmoleculecomponent.py b/gufe/components/explicitmoleculecomponent.py index fe5e7c59..c0816c26 100644 --- a/gufe/components/explicitmoleculecomponent.py +++ b/gufe/components/explicitmoleculecomponent.py @@ -41,6 +41,9 @@ def _check_partial_charges(mol: RDKitMol) -> None: """ Checks for the presence of partial charges. + Note: + We ensure the charges are set as atom properties as well to make sure they are detected by OpenFF + Raises ------ ValueError @@ -68,6 +71,12 @@ def _check_partial_charges(mol: RDKitMol) -> None: f"RDKit formal charge {Chem.GetFormalCharge(mol)}") raise ValueError(errmsg) + # set the charges on the atoms if not already set + for i, charge in enumerate(p_chgs): + atom = mol.GetAtomWithIdx(i) + if not atom.HasProp("PartialCharge"): + atom.SetDoubleProp("PartialCharge", charge) + if np.all(np.isclose(p_chgs, 0.0)): wmsg = (f"Partial charges provided all equal to " "zero. These may be ignored by some Protocols.") diff --git a/gufe/tests/test_smallmoleculecomponent.py b/gufe/tests/test_smallmoleculecomponent.py index a15c887f..ddec4a76 100644 --- a/gufe/tests/test_smallmoleculecomponent.py +++ b/gufe/tests/test_smallmoleculecomponent.py @@ -264,6 +264,23 @@ def test_partial_charges_too_few_atoms(self): with pytest.raises(ValueError, match="Incorrect number of"): SmallMoleculeComponent.from_rdkit(mol) + def test_partial_charges_applied_to_atoms(self): + """Make sure that charges set at the molecule level are transferred to atoms and picked up by openFF.""" + mol = Chem.AddHs(Chem.MolFromSmiles("C")) + Chem.AllChem.Compute2DCoords(mol) + # add some fake charges at the molecule level + mol.SetProp('atom.dprop.PartialCharge', '-1 0.25 0.25 0.25 0.25') + matchmsg = "Partial charges have been provided" + with pytest.warns(UserWarning, match=matchmsg): + ofe = SmallMoleculeComponent.from_rdkit(mol) + # convert to openff and make sure the charges are set + off_mol = ofe.to_openff() + assert off_mol.partial_charges is not None + # check ordering is the same + rdkit_mol_with_charges = ofe.to_rdkit() + for i, charge in enumerate(off_mol.partial_charges.m): + assert rdkit_mol_with_charges.GetAtomWithIdx(i).GetDoubleProp("PartialCharge") == charge + @pytest.mark.parametrize('mol, charge', [ ('CC', 0), ('CC[O-]', -1), From 28d68d0019240a8443d3dfd58ea7c12f7e1e4d17 Mon Sep 17 00:00:00 2001 From: Josh Horton Date: Thu, 6 Jun 2024 17:22:19 +0100 Subject: [PATCH 2/5] fix pep8 issues --- gufe/components/explicitmoleculecomponent.py | 3 ++- gufe/tests/test_smallmoleculecomponent.py | 20 ++++++++++++-------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/gufe/components/explicitmoleculecomponent.py b/gufe/components/explicitmoleculecomponent.py index c0816c26..abc93bda 100644 --- a/gufe/components/explicitmoleculecomponent.py +++ b/gufe/components/explicitmoleculecomponent.py @@ -42,7 +42,8 @@ def _check_partial_charges(mol: RDKitMol) -> None: Checks for the presence of partial charges. Note: - We ensure the charges are set as atom properties as well to make sure they are detected by OpenFF + We ensure the charges are set as atom properties + to ensure they are detected by OpenFF Raises ------ diff --git a/gufe/tests/test_smallmoleculecomponent.py b/gufe/tests/test_smallmoleculecomponent.py index ddec4a76..6df45728 100644 --- a/gufe/tests/test_smallmoleculecomponent.py +++ b/gufe/tests/test_smallmoleculecomponent.py @@ -265,7 +265,10 @@ def test_partial_charges_too_few_atoms(self): SmallMoleculeComponent.from_rdkit(mol) def test_partial_charges_applied_to_atoms(self): - """Make sure that charges set at the molecule level are transferred to atoms and picked up by openFF.""" + """ + Make sure that charges set at the molecule level + are transferred to atoms and picked up by openFF. + """ mol = Chem.AddHs(Chem.MolFromSmiles("C")) Chem.AllChem.Compute2DCoords(mol) # add some fake charges at the molecule level @@ -273,13 +276,14 @@ def test_partial_charges_applied_to_atoms(self): matchmsg = "Partial charges have been provided" with pytest.warns(UserWarning, match=matchmsg): ofe = SmallMoleculeComponent.from_rdkit(mol) - # convert to openff and make sure the charges are set - off_mol = ofe.to_openff() - assert off_mol.partial_charges is not None - # check ordering is the same - rdkit_mol_with_charges = ofe.to_rdkit() - for i, charge in enumerate(off_mol.partial_charges.m): - assert rdkit_mol_with_charges.GetAtomWithIdx(i).GetDoubleProp("PartialCharge") == charge + # convert to openff and make sure the charges are set + off_mol = ofe.to_openff() + assert off_mol.partial_charges is not None + # check ordering is the same + rdkit_mol_with_charges = ofe.to_rdkit() + for i, charge in enumerate(off_mol.partial_charges.m): + rdkit_atom = rdkit_mol_with_charges.GetAtomWithIdx(i) + assert rdkit_atom.GetDoubleProp("PartialCharge") == charge @pytest.mark.parametrize('mol, charge', [ From 5e8be983373892066835db049039465757d2e97d Mon Sep 17 00:00:00 2001 From: Josh Horton Date: Tue, 11 Jun 2024 10:54:40 +0100 Subject: [PATCH 3/5] Update gufe/components/explicitmoleculecomponent.py update doc string style. Co-authored-by: Irfan Alibay --- gufe/components/explicitmoleculecomponent.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/gufe/components/explicitmoleculecomponent.py b/gufe/components/explicitmoleculecomponent.py index abc93bda..21bc88a0 100644 --- a/gufe/components/explicitmoleculecomponent.py +++ b/gufe/components/explicitmoleculecomponent.py @@ -41,9 +41,10 @@ def _check_partial_charges(mol: RDKitMol) -> None: """ Checks for the presence of partial charges. - Note: - We ensure the charges are set as atom properties - to ensure they are detected by OpenFF + Note + ---- + We ensure the charges are set as atom properties + to ensure they are detected by OpenFF Raises ------ From 6d3b1db9f5a14f0533d1a3005de49feb70301121 Mon Sep 17 00:00:00 2001 From: Josh Horton Date: Mon, 17 Jun 2024 09:54:26 +0100 Subject: [PATCH 4/5] add atom charge consistent check --- gufe/components/explicitmoleculecomponent.py | 5 +++++ gufe/tests/test_smallmoleculecomponent.py | 17 +++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/gufe/components/explicitmoleculecomponent.py b/gufe/components/explicitmoleculecomponent.py index 21bc88a0..d24802b2 100644 --- a/gufe/components/explicitmoleculecomponent.py +++ b/gufe/components/explicitmoleculecomponent.py @@ -78,6 +78,11 @@ def _check_partial_charges(mol: RDKitMol) -> None: atom = mol.GetAtomWithIdx(i) if not atom.HasProp("PartialCharge"): atom.SetDoubleProp("PartialCharge", charge) + else: + atom_charge = atom.GetDoubleProp("PartialCharge") + if not np.isclose(atom_charge, charge): + errmsg = f"non-equivalent partial charges between atom and molecule properties: {atom_charge} {charge}" + raise ValueError(errmsg) if np.all(np.isclose(p_chgs, 0.0)): wmsg = (f"Partial charges provided all equal to " diff --git a/gufe/tests/test_smallmoleculecomponent.py b/gufe/tests/test_smallmoleculecomponent.py index 6df45728..89f9a4f2 100644 --- a/gufe/tests/test_smallmoleculecomponent.py +++ b/gufe/tests/test_smallmoleculecomponent.py @@ -285,6 +285,23 @@ def test_partial_charges_applied_to_atoms(self): rdkit_atom = rdkit_mol_with_charges.GetAtomWithIdx(i) assert rdkit_atom.GetDoubleProp("PartialCharge") == charge + def test_inconsistent_charges(self, charged_off_ethane): + """ + An error should be raised if the atom and molecule level + charges do not match. + """ + mol = Chem.AddHs(Chem.MolFromSmiles("C")) + Chem.AllChem.Compute2DCoords(mol) + # add some fake charges at the molecule level + mol.SetProp('atom.dprop.PartialCharge', '-1 0.25 0.25 0.25 0.25') + # set different charges to the atoms + for atom in mol.GetAtoms(): + atom.SetDoubleProp("PartialCharge", 0) + + with pytest.raises(ValueError, match="non-equivalent partial charges between atom and molecule properties"): + SmallMoleculeComponent.from_rdkit(mol) + + @pytest.mark.parametrize('mol, charge', [ ('CC', 0), ('CC[O-]', -1), From 37129a2880e17d34bf885726d0afe21082ff2288 Mon Sep 17 00:00:00 2001 From: Josh Horton Date: Mon, 17 Jun 2024 10:37:51 +0100 Subject: [PATCH 5/5] fix pep8 lines --- gufe/components/explicitmoleculecomponent.py | 3 ++- gufe/tests/test_smallmoleculecomponent.py | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/gufe/components/explicitmoleculecomponent.py b/gufe/components/explicitmoleculecomponent.py index d24802b2..a3a513e5 100644 --- a/gufe/components/explicitmoleculecomponent.py +++ b/gufe/components/explicitmoleculecomponent.py @@ -81,7 +81,8 @@ def _check_partial_charges(mol: RDKitMol) -> None: else: atom_charge = atom.GetDoubleProp("PartialCharge") if not np.isclose(atom_charge, charge): - errmsg = f"non-equivalent partial charges between atom and molecule properties: {atom_charge} {charge}" + errmsg = (f"non-equivalent partial charges between atom and " + f"molecule properties: {atom_charge} {charge}") raise ValueError(errmsg) if np.all(np.isclose(p_chgs, 0.0)): diff --git a/gufe/tests/test_smallmoleculecomponent.py b/gufe/tests/test_smallmoleculecomponent.py index 89f9a4f2..9341824e 100644 --- a/gufe/tests/test_smallmoleculecomponent.py +++ b/gufe/tests/test_smallmoleculecomponent.py @@ -298,7 +298,10 @@ def test_inconsistent_charges(self, charged_off_ethane): for atom in mol.GetAtoms(): atom.SetDoubleProp("PartialCharge", 0) - with pytest.raises(ValueError, match="non-equivalent partial charges between atom and molecule properties"): + # make sure the correct error is raised + msg = ("non-equivalent partial charges between " + "atom and molecule properties") + with pytest.raises(ValueError, match=msg): SmallMoleculeComponent.from_rdkit(mol)