Skip to content

[DRAFT] RFC Bonds orders perception - #1855

Draft
papillot wants to merge 10 commits into
molstar:masterfrom
papillot:bonds-perception
Draft

[DRAFT] RFC Bonds orders perception#1855
papillot wants to merge 10 commits into
molstar:masterfrom
papillot:bonds-perception

Conversation

@papillot

Copy link
Copy Markdown
Collaborator

Description

This PR is a WIP. POC for the Mol* implementation of Roger Sayle's algorithm for bond perception https://www.daylight.com/meetings/mug01/Sayle/m4xbondage.html which is mentioned in different bug reports #36 #626 #1361

image

The PR is not ready yet: the code has been partly vibe coded: it requires simplifications and thorough review. Also, the validation has only started and the implementation is incomplete (inter-unit bonds are not considered as of now, behavior with ).

While working on these tasks, I would like advices about the integration in Mol* codebase:

  • Parameter for bond order perception: for now it's optional, enabled by default but only kicking when there is no known bond order connectivity (not conn_bond records, or not a canonical-templated residue for which there is a table)
  • tests: unit tests have been added (they will be reviewed and cleaned), but more importantly validation test. These rely on a manifest file (committed) which lists ligands codes and PDB codes to download legacy PDB and extract the ligand coordinates only as well as the CCD cif as the source of truth (not committed). As of now, all this code is buried within the src/mol-model/**/bonds source tree. Shall it move under src/tests?
  • integration with the other chemistry aware code: this code assigns geometries to atom, computes aromaticity, matches some functional groups with charges, etc.. It should simplify heuristics and remove some assumptions that are made in the ValenceModel. For now, those assignments are lost after completion.

Actions

  • Added description of changes to the [Unreleased] section of CHANGELOG.md
  • Updated headers of modified files
  • Added my name to package.json's contributors
  • (Optional but encouraged) Improved documentation in docs

papillot and others added 10 commits June 19, 2026 18:08
This is based on an algorithm authored by Roger Sayle. Optionally applied when connectivity is not available.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
build-data.mjs downloads the pdb and CCD cifs. bonds orders and aromaticity are extracted from the cifs (source of truth). ligand extracted from the PDB are processed and compared with the results from CCD.
The data files are not commited to the repo. Only useful for validation purposes.
Script that downloads PDB files to extract ligands and compares bond assignments with the CCD cif

There is some logic to take into account equivalent terminal atoms.
Double bonds in aromatic rings have a lso a tolerance as long as the ring is aromatic
This allows overcome false positive or triage issues
@arose
arose marked this pull request as draft June 22, 2026 05:53
@arose

arose commented Jun 22, 2026

Copy link
Copy Markdown
Member

this is a tricky one

validation tests should be separate from the spec tests

unless we move chemistry perception into structure creation (which would be a very big change I am not sure about) I feel this should better be calculate property like the other chemistry perception stuff and you would need to explicitly opt-in to use it instead of the given bond-orders in unit.bonds. For sure, more cumbersome but much less obtrusive.

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.

2 participants