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

new find_mcs function for exclusion of hydrogens #110

Merged
merged 10 commits into from
Apr 14, 2023

Conversation

JohannesKarwou
Copy link
Member

@JohannesKarwou JohannesKarwou commented Mar 13, 2023

Currently, hydrogens are considered when looking for a common core with the _find_mcs function. This is not ideal, since sometimes common cores are found based on these hydrogens, where it is necessary to turn off more heavy atoms than necessary. Thanks to @jalhackl, we now have an advanced algorithm for finding the CC
An example is shown here for 2cpi -> 7cpi, with the old function on the lower part (ccor mit hdrogens) and the new algorithm on the upper part (ccor ohne hydrogens):
image

The main advantage of the new algorithm, is that some common cores might be bigger (containing more heavy atoms, we don't care about more hydrogens beeing in the dummy region) and that the error reduces drastically.
I excluded two tests (test_proposed_mutation_terminal_dummy_real_atom_match() and test_find_connected_dummy_regions1()) for the mutation 2CPI -> 7CPI, which do not make sens anymore, I suggest replacing them.

Todo

  • Write some tests for the new function (usingl the 2CPI -> 7CPI mutation)

@JohannesKarwou
Copy link
Member Author

@akaupang, found this interesting case, where our current _find_mcs algorithm is not working properly.
With the old algorithm, this common core is found:

Ligand 1 Ligand 2
image image

The position of the methoxy group is changing but according to transformato it's the same common core. With the new algorithm, the correct common core is found:

new Algorithm:

Ligand 1 Ligand 2
image image

I added this system as a test case to our transformato_testsystems repo and included a new test.

@JohannesKarwou JohannesKarwou requested a review from wiederm April 4, 2023 16:38
Copy link
Member

@wiederm wiederm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are passing and the code looks reasonable, but haven't checked in detail.

@JohannesKarwou JohannesKarwou merged commit 8eb4ceb into master Apr 14, 2023
@JohannesKarwou JohannesKarwou deleted the common_core_search branch April 14, 2023 11:14
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

Successfully merging this pull request may close these issues.

3 participants