Skip to content

Tom's rewrite#36

Open
Tomodovodoo wants to merge 50 commits intoMoleculeResolver:mainfrom
Tomodovodoo:tom_rewrite
Open

Tom's rewrite#36
Tomodovodoo wants to merge 50 commits intoMoleculeResolver:mainfrom
Tomodovodoo:tom_rewrite

Conversation

@Tomodovodoo
Copy link

Implemented the plan from our meeting in september:
context manager is lifecycle-only
service logic is abstracted behind adapters
exhaustive all-name/all-service search with grouping is supported
best-structure selection now uses deterministic RDKit stereo/chirality scoring (no SMILES-length heuristic)
strict OPSIN isomer validation is available
cross-service evidence/correlation output is explicit
default usage remains backward-compatible.

API changes:

  • search_strategy added (default first_hit)
  • resolution_mode added (default legacy)
  • include_evidence added for crosschecked search
  • include_evidence=True returns ResolutionResult
  • exhaustive searches all names/services
  • strict_isomer enforces OPSIN stereo match
  • legacy and consensus currently share the same path
  • Existing calls still work unchanged

Added features:

  • Lifecycle-only context manager
  • Service adapter ABC + registry
  • Per-identifier adapter resolution (resolve_one)
  • Exhaustive candidate dedupe/grouping
  • Deterministic consensus ranking
  • RDKit stereo/chirality-based scoring
  • Explicit tie-break ordering
  • Evidence model (CandidateEvidence)
  • Correlation fields (service/identifier/synonym)
  • New scoring/search/adapter tests
  • New scoring documentation

@Tomodovodoo
Copy link
Author

From my notes back then:

More maintainable.

Usually to clean up stuff, context managers. Not meant for logic

molecule resolver contextmanager should be rewritten

Abstract Base Class

Algorithm for best structure;
Implement isomer specific, if name == except for isomer.
OPSIN already does this.

Optional; Detailed/isomer by running through Opsin. If isomeric; OPSIN check.

5 names -> search 5 names on all service, for first found name, return.
Might be value in searching all names, getting all results, grouping, parse.

Correlations between databases.

@Tomodovodoo
Copy link
Author

Figured I might as well :P

@simonmb
Copy link
Contributor

simonmb commented Mar 2, 2026

Hi Tom, I will have to take some time to review this. :) It's a lot of changes. Thanks! I will try to get back to you in 1-2 weeks.

@Tomodovodoo
Copy link
Author

Great, that's perfect. If there's anything missing, wanted, or necessary to change, just let me know or send me an email. Don't have a lot of free time anymore (I work as an AI-engineer at the moment, with some other projects going on), but enough to work on this when I want to in my free time.

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