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

Strengthen CI and add support for rdkit>=2022.09.1 #58

Merged
merged 6 commits into from
Jun 15, 2023

Conversation

kmaziarz
Copy link
Collaborator

As discussed in #57, rdkit introduced a breaking change in 2022.09.1 that changes the return type of GetSSSR from an integer (number of rings) to the rings themselves, which broke the featurization in chem/topology_features.py. This PR makes several changes around this:

  • Patching GetSSSR to behave differently depending on rdkit version, so that the downstream code works across all versions (fixes Large amount of error messages when using decode #57).
  • Adding test_cli.py which downloads the pretrained MoLeR checkpoint and uses it in CLI to verify that the behaviour (i.e. encodings of particular molecules or samples produced through random sampling) did not change (fixes Add tests covering CLI and the model wrappers #12).
  • Extending CI to test under a range of dependency versions, starting with the original versions used when developing MoLeR (python==3.7.7, tensorflow==2.1.0 and rdkit==2020.09.1.0) and finishing with modern ones (python=3.10 and latest versions of tensorflow and rdkit).

With extended CI and the new CLI test, we can now be reasonably sure MoLeR works reliably across different dependency versions, and that the checkpoint trained under old versions continues to work in exactly the same way under modern ones.

@kmaziarz kmaziarz merged commit bd97b51 into main Jun 15, 2023
5 checks passed
@kmaziarz kmaziarz deleted the kmaziarz/update-dependencies branch June 15, 2023 16:30
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.

Large amount of error messages when using decode Add tests covering CLI and the model wrappers
2 participants