Skip to content

Fix issue #350#425

Merged
lohedges merged 1 commit intodevelfrom
fix_350
Apr 16, 2026
Merged

Fix issue #350#425
lohedges merged 1 commit intodevelfrom
fix_350

Conversation

@lohedges
Copy link
Copy Markdown
Contributor

This PR closes #350 by preferentially using RDKit::determineBondOrders() to infer bond orders when missing, only failing back to the existing (MDAnalysis-baed) inference routine when this fails. A unit test confirms that the problem inputs from the issue thread convert correctly when using SDF or AMBER inputs, i.e. the SMILES strings of the convert RDKit molecules match. All existing RDKit conversion tests pass, as do extended tests in BioSimSpace.

  • I confirm that I have merged the latest version of devel into this branch before issuing this pull request (e.g. by running git pull origin devel): [y]
  • I confirm that I have added a changelog entry to the changelog (we will add a link to this PR as part of the review): [y]
  • I confirm that I have permission to release this code under the GPL3 license: [y]

@lohedges lohedges added bug Something isn't working cresset related to work with cresset labels Apr 16, 2026
Copy link
Copy Markdown
Collaborator

@tom-potter-cresset tom-potter-cresset left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this fixes the problem case I reported

@lohedges lohedges merged commit a7c3fd9 into devel Apr 16, 2026
5 checks passed
@lohedges lohedges deleted the fix_350 branch April 16, 2026 16:53
lohedges added a commit that referenced this pull request Apr 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working cresset related to work with cresset

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Incorrect charges from Sire to RDKit conversion

2 participants