Conversation
…and chemical perception
…ers, and hybridization states
…perDihedral structures
…tures for enhanced molecular perception
… and bond validation
…harge and lone pair assignment
… hybridization state assignments
…ent classification
…ne and its stages
…itrogen and carbon in ring structures
…eflect correct charge assignment
…gen with lone pairs adjacent to π-systems
…on behavior in carbonyl and thioester structures
…and push_resonance_system for clarity
…idinium, thiourea, and phosphate groups
…hedral constructors
…ms in verify_atom_types
…es in dreid-typer
… perception pipeline stages
…der process and its components
…ule system and its structure
27 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the core bond representation to explicitly separate input bond orders (GraphBondOrder) from output topology bond orders (TopologyBondOrder). The key innovation is replacing a generalized conjugation detection system with strict substructure matching for specific resonance motifs (carboxylates, nitro groups, amides, etc.). This ensures that aromatic and resonant systems are precisely identified and marked with TopologyBondOrder::Resonant in the final topology.
Key Changes:
- Introduced dual bond order types:
GraphBondOrder(input) andTopologyBondOrder(output) withResonantreplacingAromatic - Replaced generalized Pauling-library-based resonance detection with targeted functional group matchers
- Enhanced test harness to verify both input and expected output bond orders for all integration tests
Reviewed changes
Copilot reviewed 26 out of 28 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/core/properties.rs |
Added GraphBondOrder and TopologyBondOrder enums with appropriate parsing |
src/core/topology.rs |
New file defining output topology structures using TopologyBondOrder |
src/core/mod.rs |
Added topology module export |
src/perception/resonance.rs |
Complete rewrite replacing Pauling integration with strict functional group detection |
src/perception/model.rs |
Added ResonanceSystem struct, removed Pauling trait implementations |
src/perception/kekulize.rs |
Updated to use GraphBondOrder, added has_aromatic_edge tracking |
src/perception/hybridization.rs |
Enhanced with iterative resonance propagation logic |
src/perception/mod.rs |
Updated tests to check is_resonant flag |
tests/harness.rs |
Split into InputBondBlueprint and OutputBondBlueprint with verification |
tests/cases/*.rs |
Comprehensive updates adding expected bond orders for all test cases |
src/lib.rs |
Updated documentation examples and public API exports |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
Refactors the core bond representation to explicitly distinguish between input bond orders (
GraphBondOrder) and final topology bond orders (TopologyBondOrder). This decoupling allows for precise control over how aromatic and resonant systems are perceived and represented in the output. Additionally, the integration test suite has been significantly enhanced to strictly verify these final bond orders, ensuring that resonance systems (like carboxylates, amides, and aromatic rings) are correctly identified and assigned theResonantbond order.Changes:
Decoupled Bond Order Types:
GraphBondOrder(Single, Double, Triple, Aromatic) for inputMolecularGraphconstruction.TopologyBondOrder(Single, Double, Triple, Resonant) for outputMolecularTopology.add_bond,Bond) to use these specific types, preventing ambiguity.Implemented Strict Resonance Perception:
TopologyBondOrder::Resonant.Enhanced Test Suite:
MoleculeTestCaseharness to includeexpected_bonds, allowing rigorous verification of final bond orders.GraphBondOrderand expected outputTopologyBondOrder.Diglycineto verify peptide bond resonance handling.Documentation and Examples:
README.mdand crate-level documentation to reflect the newGraphBondOrderAPI and provide correct usage examples.01_pipeline.md) to clarify the distinction between graph and topology representations.