openfreeenergy / kartograf Goto Github PK

First of all, thanks for the nice package!

While trying it out I encountered two problems after installing it with python -m pip install kartograph:

1. The package installed didn't have dill in the list of dependencies (I see that's fixed now on the pyproject.toml);
2. Installing kartograph via pip also installed an outdated version of gufe (0.4), which didn't have some functions used by this package.

One thing that could be done is to add information on the README file that the packages can also be installed directly from their respective GitHub repos, as in:

python -m pip install git+https://github.com/OpenFreeEnergy/gufe.git
python -m pip install git+https://github.com/OpenFreeEnergy/kartograf.git

The drawback of this approach is that having this listed on the pyproject.toml as an installation strategy prevents the package from being uploaded to PyPI.

Please let me know if that's something you'd like to have, happy to make a PR.

everything an installable package needs

• Depends on OpenFE integration as well.
• pypi installer
• conda?
• docs?
• tests

Filters are cool, let's document how they're used and how to inject them.

we've run into a few problematic mappings with the kartograf mapper for fused ring systems. kartograf maps the following example (red are atoms mapped to remove):

But with single/hybrid topology methods this won't work. Instead, we would want a mapping like this:

which is of course not very efficient but with enough sampling is much more likely to give an accurate DDG estimate. If you want to reproduce our particular transformation you can use SMILES c1ccc(cc1)COC(=O)Nc2cc(cc(c2)Cl)C(=O)Nc3ccc4c(c3)CC[C@@H]4N and c1ccc(cc1)COC(=O)Nc2cc(cc(c2)Cl)C(=O)Nc3cccc(c3)[C@H]4CCNC4.

There are some implementations on allow ring breaking options for popular mappers, see here and here for some examples. Would it be in scope for kartograf to have this option too? Currently we need to revert back to perses mappers/scorers for this kind of transformation.

Is your feature request related to a problem? Please describe.
Adding a filter, that allows to inhibit non-heav to heavz atom changes, could be useful for some folks!

Describe the solution you'd like
There should be a filter function no heavy-to-nonheavy

Additional context
some FE code, can not handle the bonded change.

Describe the bug
When trying to suggest_multistate_mappings for an array of components. It automatically tries the new greedy algorithm. On two different systems, we've encountered the following bug

-> Setting an array element with a sequence for array a1_d.

It works if we set greedy to false.

To Reproduce
Steps to reproduce the behavior:

1. Switch to "expanding for multistates" branch
2. Trying out the following code snippet

`from kartograf import KartografAtomMapper, SmallMoleculeComponent
from rdkit import Chem

mapper = KartografAtomMapper()

ligands = [mol for mol in Chem.SDMolSupplier('set_A.sdf')]
components = [SmallMoleculeComponent(mol) for mol in ligands]
print(components)
mapping = mapper.suggest_multistate_mapping(components, greedy=False)

import json

with open('mapping.json', 'w') as f:
json.dump(mapping, f)`

4. We used for set_A following sdf file
   set_A.txt

5. Run

Expected behavior
Should provide a mapping for all ligands

Desktop (please complete the following information):

• OS: Ubuntu
• Version: newest version of Kartograf

If I can or should provide with any further information, let me know. Thank you very much for providing such helpful modules!

In some mappings in ligands in the PLB system, mapping atoms that are connected by single or double bonds can lead to problems. It would be great to see if mappings that do not map these hybridization changes would perform better.

Example 1: P38 Edge lig_p38a_2gg - lig_p38a_2ee

Mapping of sulfone and amide let to "stereo inversion" with OpenMM 8.0.0

Other example:
CDK8 Lig_25 - lig_24

Multistate methods are upcoming methods for FE calculations. Kartograf can also support such methods with the approch in the linked PR #12 .

Describe the bug
A clear and concise description of what the bug is.

function filter_atoms_h_only_h_mapped filters mapped heavy atoms where they are different elements.

In this condition, only same elements including heavy atoms are mapped. But this function should only filter hydrogen <-> heavy atoms according to function name and doc.

    if (atomA.GetAtomicNum() == atomB.GetAtomicNum() == 1) or (
        atomA.GetAtomicNum() == atomB.GetAtomicNum() != 1
    ):

https://github.com/OpenFreeEnergy/kartograf/blob/ea612b10354a0e162e7152be9aebfaed76fe48ef/src/kartograf/atom_mapper.py#L65C1-L66C1

I think this is going nowhere! I should wire it like the H-Heavy Atom mapping code.

When using the Kartograf atom mapper for the shp2 benchmarking system (https://github.com/OpenFreeEnergy/protein-ligand-benchmark), some mappings include the mapping of ring systems that have a different hybridization. Would it be possible to add a filter that disallows such mappings?

• pypi release
• conda-forge release conda-forge/staged-recipes#22026

We can save some time in our CI by adding a channel to our environment.yml that will prevent us from pulling in cudatoolkit