-
Notifications
You must be signed in to change notification settings - Fork 48
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
fix zero division error in conformers #209
Conversation
Thanks, @shuyana and nice catch for the bug! Do you mind adding a test for it? |
Codecov Report
@@ Coverage Diff @@
## main #209 +/- ##
=======================================
Coverage 91.94% 91.94%
=======================================
Files 46 46
Lines 3826 3826
=======================================
Hits 3518 3518
Misses 308 308
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Thank you for the quick reply! def test_conformer_no_rotatable_bonds():
mol = dm.to_mol("c1ccccc1")
random.seed(42)
np.random.seed(42)
mol1 = dm.conformers.generate(mol, minimize_energy=True)
random.seed(42)
np.random.seed(42)
mol2 = dm.conformers.generate(mol, minimize_energy=True, eratio=3)
# `eratio` should be ignored for this molecule as it has no rotatable bonds
assert mol1.GetNumConformers() == mol2.GetNumConformers() This test fails without the fix because If you have any comments or suggestions, please let me know! |
Thanks. That sounds good to me. I am good to merge here once the |
Thanks for the review! |
Changelogs
Fixed a division-by-zero error in
dm.conformers.generate()
whenminimize_energy
is set toTrue
and the molecule has no rotatable bonds. Below is a minimal example that reproduces the error:Error message:
Checklist:
feature
,fix
ortest
(or ask a maintainer to do it for you).discussion related to that PR