Feature merge2 #183
Merged
Feature merge2 #183
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…tomMapping objects to mutate or merge molecules.
…ed at both end states. This means we now have the full information in this object to be able to properly construct merged and mutated molecules. Added a unit test to verify this works for mapping an alanine to a lysine within a protein (mapping works on subsets of molecules, meaning we can do a region of interest mapping and merge just be passing in the residues that we want to map - see test_match.py) Also, removed the C++ code for mutate as I realised that mutation is a merge followed by extracting the perturbed state. This is still a WIP - have got matching working, and have added the necessary info to AtomMapping. The next step is to fill in the C++ code for merge based on the information in AtomMapping.
…ntial energy of internal terms via the Cursor
Gone a little down the rabbit hole in the merge code by adding in support for alternate names for atoms and residues. This will enable the names of the perturbed end state to be saved, so that we can get something sensible out when we extract the end states. Will not compile
…ernate atom and residue names. Needs testing, plus then integrating with the merge code
Added in code for fix_170
…t hole and ready to go back to the merge code ;-)
…to store the mappings - and then this could be passed to the properties so that they can update themselves internally
Working on the code to move the merge into the properties themselves (where they know what they are doing)
…ters. Also beginning to fill in the various merge functions
…e, so that we can emit debug, log, warning and error messages from C++ that correctly get printed up in the Python layer (and can be properly customised and controlled in the Python layer). This is the first step to consolidating all of the messaging into a single Python logging-compatible framework (i.e. also removing some of the rich.console layer)
…Properties, plus have generated the new wrappers. Mostly compiling now - just need to fill in missing atomidxmapping.h includes in the wrappers.
…ses, and have now added all of the python wrappers. This all compiles, links and passes unit tests. Next step is to actually write the underlying merge code...
Next onto internal merging
…merge the connectivities together, as well as create a single merged connectivity.
…ity object. Testing it...
…ok like they are merging properly :-)
…st only in the perturbed state are copied into the reference state
…re now merging angles, dihedrals and impropers. But this all needs testing. Will do that after I have added in the code to merge the CLJNBPairs
…bonds/angles/dihedrals for atoms unmapped in the perturbed state copied from the match in the reference state
|
I've also implemented the This is now finished - nothing else will go into 2024.1.0 ;-) |
|
Windows build failed because it couldn’t install kcombu_bss - I guess a timeout or conda issue. Will rerun once the MacOS build has finished. |
lohedges
approved these changes
Apr 8, 2024
There was a problem hiding this comment.
Many thanks, this is fantastic. I really like how it's now possible to get so much information about the underlying perturbation. This will make it much easier for the user to understand what's going on, as well as massively simplifying debugging.
It's a real shame the GitHub's reStructuredText rendering is so broken at the moment since it makes it really hard to follow the tutorials in the repo. (The website will be fine.) The only thing that I spotted while reading through is an inconsistency in the option used to allow matching of light atoms in a mapping.
In this part of the tutorial it says:
By default, this algorithm ignores hydrogens. This is why only a single atom was matched above. You can change this by passing in the
ignore_light_atoms=Trueargument.
In a later section you use:
mapping = sr.morph.match(ala, lys, match_light_atoms=True) I assume that this is meant to be match_light_atoms in both cases.
|
Just to say that I'm seeing the following warning when testing some free-energy simulations locally: ────────────────────────────────────────────────────────────────── WARNING ───────────────────────────────────────────────────────────────────
Could not convert the old property at key parameters with type SireMM::AmberParams to match the new, edited molecule. This property has been
deleted.
Error was SireError::incomplete_code: Cannot make compatible if atom order has changed!
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────I'll investigate why this is happening. (This is on my |
|
Everything still runs as expected, only I now see the warning above. This is definitely something that Exscientia would see. I'll try to figure out what bit of code is triggering it. I assume this is when the system is squashed, i.e. the two end states and put one after the other in another system. |
|
Yes, that was it. Running the following: Gives: ============================================================ test session starts ============================================================
platform linux -- Python 3.11.7, pytest-7.4.4, pluggy-1.3.0 -- /home/lester/.conda/envs/openbiosim/bin/python
cachedir: .pytest_cache
rootdir: /home/lester/Code/openbiosim/biosimspace
configfile: pyproject.toml
plugins: anyio-4.2.0
collected 28 items
tests/Sandpit/Exscientia/Align/test_squash.py::test_squash[False-expected_n_atoms0]
────────────────────────────────────────────────────────────────── WARNING ──────────────────────────────────────────────────────────────────
Could not convert the old property at key parameters with type SireMM::AmberParams to match the new, edited molecule. This property has been
deleted.
Error was SireError::incomplete_code: Cannot make compatible if atom order has changed!
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
────────────────────────────────────────────────────────────────── WARNING ──────────────────────────────────────────────────────────────────
Could not convert the old property at key parameters with type SireMM::AmberParams to match the new, edited molecule. This property has been
deleted.
Error was SireError::incomplete_code: Cannot make compatible if atom order has changed!
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
PASSED
tests/Sandpit/Exscientia/Align/test_squash.py::test_squash[True-expected_n_atoms1] PASSED
tests/Sandpit/Exscientia/Align/test_squash.py::test_squash_multires[False]
────────────────────────────────────────────────────────────────── WARNING ──────────────────────────────────────────────────────────────────
Could not convert the old property at key parameters with type SireMM::AmberParams to match the new, edited molecule. This property has been
deleted.
Error was SireError::incomplete_code: Cannot make compatible if atom order has changed!
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
────────────────────────────────────────────────────────────────── WARNING ──────────────────────────────────────────────────────────────────
Could not convert the old property at key parameters with type SireMM::AmberParams to match the new, edited molecule. This property has been
deleted.
Error was SireError::incomplete_code: Cannot make compatible if atom order has changed!
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
PASSED
tests/Sandpit/Exscientia/Align/test_squash.py::test_squash_multires[True] PASSED
tests/Sandpit/Exscientia/Align/test_squash.py::test_squashed_molecule_mapping[False] PASSED
tests/Sandpit/Exscientia/Align/test_squash.py::test_squashed_molecule_mapping[True] PASSED
tests/Sandpit/Exscientia/Align/test_squash.py::test_squashed_atom_mapping_implicit[False] PASSED
tests/Sandpit/Exscientia/Align/test_squash.py::test_squashed_atom_mapping_implicit[True] PASSED
tests/Sandpit/Exscientia/Align/test_squash.py::test_squashed_atom_mapping_explicit[False] PASSED
tests/Sandpit/Exscientia/Align/test_squash.py::test_squashed_atom_mapping_explicit[True] PASSED
tests/Sandpit/Exscientia/Align/test_squash.py::test_unsquash[False]
────────────────────────────────────────────────────────────────── WARNING ──────────────────────────────────────────────────────────────────
Could not convert the old property at key parameters with type SireMM::AmberParams to match the new, edited molecule. This property has been
deleted.
Error was SireError::incomplete_code: Cannot make compatible if atom order has changed!
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
────────────────────────────────────────────────────────────────── WARNING ──────────────────────────────────────────────────────────────────
Could not convert the old property at key parameters with type SireMM::AmberParams to match the new, edited molecule. This property has been
deleted.
Error was SireError::incomplete_code: Cannot make compatible if atom order has changed!
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
PASSED
tests/Sandpit/Exscientia/Align/test_squash.py::test_unsquash[True] PASSED
tests/Sandpit/Exscientia/Align/test_squash.py::test_unsquash_multires[False]
────────────────────────────────────────────────────────────────── WARNING ──────────────────────────────────────────────────────────────────
Could not convert the old property at key parameters with type SireMM::AmberParams to match the new, edited molecule. This property has been
deleted.
Error was SireError::incomplete_code: Cannot make compatible if atom order has changed!
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
────────────────────────────────────────────────────────────────── WARNING ──────────────────────────────────────────────────────────────────
Could not convert the old property at key parameters with type SireMM::AmberParams to match the new, edited molecule. This property has been
deleted.
Error was SireError::incomplete_code: Cannot make compatible if atom order has changed!
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
PASSED
tests/Sandpit/Exscientia/Align/test_squash.py::test_unsquash_multires[True] PASSED
tests/Sandpit/Exscientia/Align/test_squash.py::test_squashed_molecule_mapping_decouple_lambda[decoupled_system-False-True-True] PASSED
tests/Sandpit/Exscientia/Align/test_squash.py::test_squashed_molecule_mapping_decouple_lambda[decoupled_system-True-True-False] PASSED
tests/Sandpit/Exscientia/Align/test_squash.py::test_squashed_molecule_mapping_decouple_lambda[redecoupled_system-False-True-False] PASSED
tests/Sandpit/Exscientia/Align/test_squash.py::test_squashed_molecule_mapping_decouple_lambda[redecoupled_system-True-True-True] PASSED
tests/Sandpit/Exscientia/Align/test_squash.py::test_squashed_molecule_mapping_decouple_lambda[decoupled_system-False-False-False] PASSED
tests/Sandpit/Exscientia/Align/test_squash.py::test_squashed_molecule_mapping_decouple_lambda[decoupled_system-True-False-False] PASSED
tests/Sandpit/Exscientia/Align/test_squash.py::test_squashed_molecule_mapping_decouple_lambda[redecoupled_system-False-False-False] PASSED
tests/Sandpit/Exscientia/Align/test_squash.py::test_squashed_molecule_mapping_decouple_lambda[redecoupled_system-True-False-False] PASSED
tests/Sandpit/Exscientia/Align/test_squash.py::test_squashed_molecule_mapping_order[decouple-merge-normal-order0] PASSED
tests/Sandpit/Exscientia/Align/test_squash.py::test_squashed_molecule_mapping_order[decouple-normal-merge-order1] PASSED
tests/Sandpit/Exscientia/Align/test_squash.py::test_squashed_molecule_mapping_order[merge-decouple-normal-order2] PASSED
tests/Sandpit/Exscientia/Align/test_squash.py::test_squashed_molecule_mapping_order[merge-normal-decouple-order3] PASSED
tests/Sandpit/Exscientia/Align/test_squash.py::test_squashed_molecule_mapping_order[normal-merge-decouple-order4] PASSED
tests/Sandpit/Exscientia/Align/test_squash.py::test_squashed_molecule_mapping_order[normal-decouple-merge-order5] PASSED
============================================================= warnings summary ============================================================== |
|
Yes - these are properties that can't be automatically merged (and would have been silently dropped). There are a couple of solutions. One is to disable warnings while you are doing the merge (not great). Or the other is to remove the |
|
That makes sense. In this case it doesn't seem to happen during a merge, though. I can't quite figure out where exactly within the |
|
Okay, I've found where it's happening (within a private |
lohedges
added a commit
to OpenBioSim/biosimspace
that referenced
this pull request
Apr 8, 2024
|
I'll also make this change in the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the finished
feature_mergepull request, that brings in all of the functionality needed for2024.1.0, namely support for creating merge molecules, decoupling, annihilation, and better debugging of lambda schedules. I've also added in pow support for GeneralUnit.I've added all the tests, plus now fully describe the new functionality in a new part 7 of the tutorial.
Once merged, I'll get everything ready for the
2024.1.0release :-)develinto this branch before issuing this pull request (e.g. by runninggit pull origin devel): [y]Suggested reviewers:
@lohedges
Any additional context of information?
I've run all the BioSimSpace tests, plus sire_bigtests, plus checked that the ethane/methanol free energy is still correct. Not much has changed functionally since feature_merge - just some additional functionality as described in the tutorial (and also experimented with in Julien's lab).