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

RDKitConverter improvements #3996

Open
cbouy opened this issue Jan 17, 2023 · 0 comments
Open

RDKitConverter improvements #3996

cbouy opened this issue Jan 17, 2023 · 0 comments

Comments

@cbouy
Copy link
Member

cbouy commented Jan 17, 2023

Running the RDKitConverter is slow and its accuracy could always be improved.

Here are a few ideas for improvements:

Category Description Status
Speed Use RDKit's RunReactantInPlace instead of using RunReactants while transferring atom properties from the reactant mol to the product which causes a bottleneck. Merged in PR #4082
Both Sorting atoms by their number of heavy atom neighbors (ascending order) before running the inferring code should help in not having to patch conjugated systems in some cases. Merged in PR #4082
Both This line should probably exclude this case: [O-]-C(=O)
Accuracy Analyse the failing molecules from the RDKitConverterBenchmark to identify patterns and handle them with bespoke reactions. Related issue: #3339
Both RDKit 2022.09.1 added a C++ XYZParser which also includes code to infer bond orders and charges from the topology which could be reused here as an alternative implementation. The only downside is that it requires to know in advance the total charge of the molecule. But if the total charge is known, it should be much faster than our current code. Also adding a simple wrapper around AssignBondOrdersFromTemplate would be very useful. In progress in PR #4305
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