fix zero division error in conformers

[shuyana]

Changelogs

• Fixed a division-by-zero error in dm.conformers.generate() when minimize_energy is set to True and the molecule has no rotatable bonds. Below is a minimal example that reproduces the error:

  import datamol as dm
  
  mol = dm.to_mol("c1ccccc1")
  dm.conformers.generate(mol, minimize_energy=True)

  Error message:

  /workspaces/datamol/datamol/conformers/_conformers.py:214: RuntimeWarning: invalid value encountered in scalar divide
  if E - minE <= ewindow and (E - minE) / rotatable_bonds <= eratio
  /workspaces/datamol/datamol/conformers/_conformers.py:214: RuntimeWarning: divide by zero encountered in scalar divide
  if E - minE <= ewindow and (E - minE) / rotatable_bonds <= eratio

---

Checklist:

• [ ] Was this PR discussed in an issue? It is recommended to first discuss a new feature into a GitHub issue before opening a PR.
• [x] Add tests to cover the fixed bug(s) or the new introduced feature(s) (if appropriate).
• [ ] Update the API documentation is a new function is added, or an existing one is deleted.
• [x] Write concise and explanatory changelogs below.
• [x] If possible, assign one of the following labels to the PR: feature, fix or test (or ask a maintainer to do it for you).

---

discussion related to that PR

[hadim]

Thanks, @shuyana and nice catch for the bug!

Do you mind adding a test for it?

[codecov[bot]]

Codecov Report

Merging #209 (100fd4c) into main (2619c61) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #209   +/-   ##
=======================================
  Coverage   91.94%   91.94%           
=======================================
  Files          46       46           
  Lines        3826     3826           
=======================================
  Hits         3518     3518           
  Misses        308      308           

Flag	Coverage Δ
unittests	91.94% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
datamol/conformers/_conformers.py	93.16% <ø> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[shuyana]

Thank you for the quick reply! I think the existing test, test_conformer_energy(), covers the case where rotatable_bonds is not zero. So, I've added a test for the case where rotatable_bonds is zero:

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 (E - minE) / rotatable_bonds is inf and always greater than the finite eratio for mol2. On the other hand, the test passes with the fix because (E - minE) / rotatable_bonds is not used when rotatable_bonds is zero.

If you have any comments or suggestions, please let me know!

[hadim]

Thanks. That sounds good to me.

I am good to merge here once the ruff and black CI errors are gone (likely as simple as running those locally).

[shuyana]

Thanks for the review! I've fixed the CI errors and pushed the changes.