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

MMFF94 Error in setup() function #562

Open
pbrach opened this issue Jul 24, 2015 · 0 comments
Open

MMFF94 Error in setup() function #562

pbrach opened this issue Jul 24, 2015 · 0 comments

Comments

@pbrach
Copy link

pbrach commented Jul 24, 2015

I found 2 issues in MMFF94.C that correlate with ring-perception. These came up after fixing the RingPerceptionProcessor and introducing a new exception for cases where we could not correctly generate the SSSR.

In MMFF94.C, Method bool MMFF94::specificSetup(), line 177 and line 227 we call collectRings_() twice. I can find no reason for this, a single call should suffice and I suppose that two calls will lead to wrong behavior. But I am not 100% certain.

Furthermore the routine collectRings_() itself contains a bug in line 665 because here we call calculateSSSR(rings, *getSystem()).

The problem is that a system may contain several unconnected molecules, but the calculateSSSR-method only works on a single connected component. Giving a whole system as input argument we handle all individual AtomContainer/Molecules contained in the system as a single AtomContainer in the RingPerceptionProcessor. This won't work, and certainly did not work before (but at that time we had no exception in the RingPerceptionProcessor pointing the error out).

What we need to do: make sure that we fill the MMFF94 field "rings_" iteratively by appling calculateSSSR() to every connected component of the system. In my opinion we should not only iterate over all molecule-entries within the system, but should also apply a ConnectedComponentProcessor to every individual molecule-entry and use the resulting connectedComponents for ring-finding.

I feel uncomfortable fixing this myself, as I am not too familiar with the forcefields yet and have other stuff on my agenda.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants