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

Use Chemprop version 2 #14

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

davide-grheco
Copy link

This update transitions the codebase from Chemprop v1 to Chemprop v2 to address compatibility issues and enhance functionality, as discussed in Issue #11. Given that Chemprop v1 is no longer supported, this upgrade ensures access to ongoing updates and support from the Chemprop community.

Key improvements with Chemprop v2 include:

  • Direct RDKit Mol Object Support: Chemprop v2 introduces the MoleculeDataPoint class, allowing direct initialization from RDKit Mol objects and reducing the need for repetitive SMILES-to-Mol conversions, which streamlines predictions and minimizes computational demands.
  • Enhanced Consistency: By directly supporting Mol objects, this version reduces potential mismatches from converting between SMILES and Mol objects, adding reliability for workflows where Mol objects are already used.

Additionally, the code was refactored into smaller functions to improve readability and modularity. However, if preferred, these changes can be reverted to the previous structure.

Finally, it will be necessary to verify that models with Chemprop v2 perform comparably with those from v1 to ensure accuracy and consistency. Benchmark testing will be conducted to validate model outputs and adjust if needed to align with v1 performance levels.

@swansonk14
Copy link
Owner

Hi @davide-grheco,

Thank you so much for creating this PR and doing all the work to transition to Chemprop v2! I'll aim to work on checking the code and training the Chemprop v2 models in December to ensure that they maintain the same level of accuracy as the v1 models.

Best,
Kyle

@davide-grheco
Copy link
Author

Hello @swansonk14,
thank you for taking a look and performing the validation. I'll be quite busy in December, but if you have any comment or you feel something should be changed feel free to contact me here or at my email.

Greetings
Davide

@manglav
Copy link

manglav commented Jan 10, 2025

Hi! I just wanted to check in on this - would love to see if I there are performance improvements and more flexibility by upgrading to Chemprop 2.0/2.1.

@swansonk14
Copy link
Owner

Hi @manglav,

Thank you for checking on this! Unfortunately my other research projects have taken priority recently and it will take me a while to work through all of the Chemprop v2 changes. I'll attempt to take a look when I have some more time, but in the meantime, I think Chemprop v1 should still handle most use cases well.

Best,
Kyle

@davide-grheco
Copy link
Author

Dear @manglav,
I did not conduct any performance tests on chemprop v2. I do not know whether changes between the two versions were done to improve efficiency.
This one PR is not focused on improving efficiency either, but on just migrating to the new version.
Migrating to the new version is a prerequisite to add some performance improvements to admet ai. Such performance improvements however would arise only under specific conditions: if you already have the Mol object in memory and especially if you precalculated the fingerprints.
The chemprop v2 interface in fact would allow us to pass the precalculated fingerprints, which would greatly increase computation times if you precomputed them.

However, before merging it is fundamental to ensure that changes in chemprop version do not deteriorate the quality of the models. In fact I had to train new models for the new version, these use the same architecture - but you never know. I spoke with Kyle about such validation, but currently we are both quite busy and can't really perform such tests.
The validation should not be too complex to perform if you want to perform it yourself.
Greetings
Davide

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