[Autosolve PR 1] Add hallucination helpers and autosolve foundations - #229
Open
shreyasvinaya wants to merge 9 commits into
Open
[Autosolve PR 1] Add hallucination helpers and autosolve foundations#229shreyasvinaya wants to merge 9 commits into
shreyasvinaya wants to merge 9 commits into
Conversation
- hallucination_helpers: build_ml_checker, filter_with_checker, resolve_hallucination for optional ML/heuristic filtering - HallucinationChecker type alias in deepretro.utils.typing - canonicalize() in utils_molecule for canonical SMILES normalization - Full test suite for hallucination helpers (7 tests) - Update models/__init__ exports
10 tasks
- Fix filter_with_checker: unknown pathways from checker no longer cause misaligned return lengths (pathways without matching metadata are dropped) - Remove all monkeypatch and MagicMock usage — inject classifier doubles - Use real molecules (aspirin, salicylic acid, paracetamol) - Add missing edge cases: unknown pathway from checker, missing is_hallucination key defaults to filtered, empty pathways short-circuit - Remove lazy import of is_valid_smiles (top-level import) - Direct imports in models/__init__ (lazy only for deepchem-dependent modules)
Source fixes: - is_hallucination check uses `is False` (None/0/missing no longer pass) - filter_with_checker validates input length alignment (raises ValueError) - Unknown pathways from checker raise ValueError (was silent data loss) - build_ml_checker validates classifier at construction (TypeError) - HallucinationPredictor Protocol replaces Any for classifier type - Raises sections added to all public docstrings Test fixes: - Module-level test doubles (StubClassifier, SelectiveClassifier) - No private classes, no nested functions, no meta-commentary - Covers: None value, missing key, mismatched lengths, unknown pathway, classifier validation, multi-reactant join, empty input
Callers must pass a loaded model object (HallucinationPredictor) rather than a file path. This removes the implicit load-from-disk side effect and the dependency on hallucination_classifier module at resolution time.
# Conflicts: # deepretro/models/hallucination_helpers.py # deepretro/tests/test_hallucination_helpers.py
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
1 participant
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
hallucination_helpersmodule with ML/heuristic pathway filtering (build_ml_checker,filter_with_checker,resolve_hallucination)HallucinationCheckertype alias todeepretro.utils.typingcanonicalize()toutils_moleculefor canonical SMILES normalizationmodels/__init__exportsThe checker now takes an already-loaded classifier object (anything satisfying
HallucinationPredictor) instead of loading a model from a path string. This decouples the helpers from deepchem/HallucinationClassifier, makes them trivially testable with fakes, and lets callers control model lifetime. Path-based loading was intentionally removed.Type of change
Please check the option that is related to your PR.
Checklist