Repository navigation
Fix cluster parallelism with OpenEye evaluators (v1.2.0), simplify README, clean up repo - #21
Merged
Merged
Conversation
The repo had accumulated generated output and scratch files at the root and in tutorials/. CLAUDE.md already documented jobs/ as gitignored, but no rule existed, so 80 job files plus 53 generated figures showed up as untracked noise on every status check. Add ignore rules for: - jobs/ and plots/ (generated Slurm files and figures) - *.bak / *.bak2 (manual editor backups) - OpenEye run artifacts (oeomega_*.txt, *_settings.param, *_status.txt, *.oedu) Scratch files removed from the tree (preserved outside the repo): a stray 38-byte note named "4", five OpenEye log/parameter dumps, and five tutorial .bak/.bak2 backups now superseded by git history. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LookupEvaluator returned np.nan for any product code absent from the reference table, and the sampler skips NaN evaluations. That is correct when the table is a complete enumeration and a miss means missing data. It is wrong for sparse experimental libraries such as DEL read counts, where only a small fraction of the combinatorial space is measured and an absent combination is a true non-binder (score 0). Skipping those draws silently discards most of the search. Add an optional default_score to LookupEvaluator and LookupEvaluatorConfig, wired through create_evaluator. Default stays None, which preserves the existing NaN behavior, so no current run changes. A measured 0.0 is still returned from the table and is not confused with an absent code. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Boltzmann weight is exp(z) with z = (score - mean) / std / temperature. Because z is standardized it is normally O(1), but on a heavy-tailed, zero-inflated landscape (most reagents scoring 0, a few very high, as in DEL read counts) a single outlier pushes max(z) toward sqrt(n), and a small CATS temperature divides that further. exp() then overflows to inf and inf/sum(inf) = nan, so rng.choice raises "probabilities contain NaN" and the search dies mid-run. Subtract max(z) before exponentiating in both the select_reagent and select_batch paths. This is shift-invariant, so the normalized probabilities are mathematically identical, but every exponent is now <= 0 and the largest weight is exp(0) = 1, making overflow impossible. Regression tests confirmed to fail against the previous naive form. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
TopTwoSelection._calculate_gmic returned 0.0 whenever the least-observed active reagent had fewer than min_observations samples. RouletteWheelSelection never had this gate, so the two strategies computed criticality differently. On large components the gate fired almost every cycle: a single sub-5-obs straggler among hundreds of reagents pinned the whole component's GMIC to zero, which then over-weighted that component in the GMIC rotation. A scan found the gate firing on 25 of 28 benchmark libraries. A paired counterfactual (20 replicates, monkeypatched, shipped code untouched) showed removing it lifts adenine TT-TS top-100 recovery from 87.1 to 93.2 and cuts replicate variance roughly in half (sd 18.4 -> 10.6), rescuing 3 of 4 bimodal failures, with no change on the 3 libraries where the gate never fired. min_observations is now inert in TopTwoSelection. It is still accepted and stored so existing configs keep working, and both docstrings now say so rather than describing a gate that no longer exists. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds diagnostic plots that make the criticality-directed search visible as a signal -> decision -> result chain, so a run can be inspected rather than inferred from recovery numbers alone. Layer 1 (which component to explore): - plot_gmic_directed_exploration: per-component GMIC (signal) -> heated component strip (decision) -> cumulative per-component coverage (result) - plot_gmic_vs_oracle / compute_oracle_flexibility: GMIC against a ground-truth flexibility reference Layer 2 (how hard to explore within that component): - plot_adaptive_intensity: the per-component intensity knob as a multiplier centered on 1.0 (RWS cats_multiplier, TT-TS adaptive heated_scale) -> rolling heating probability -> new reagents per batch, with automatic detection of which intensity column the run recorded - plot_reagent_usage_action_panel: per-batch reagent usage against the activity cutoff, with component names inferred from reagent files Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fixes the three defects from the 2026-06-23 bug report, which together made processes > 1 non-functional with Fred docking out of the box. Bug 1 -- parallelism could not be enabled. from_config() hardcoded processes=1, and ThompsonSamplingConfig had no field for it, so there was no API-level way to turn on evaluation parallelism. A 1000-iteration docking screen ran on one core of a 128-core allocation, with no error or warning. Add processes and min_cpds_per_core to the config and pass them through from_config(). Bug 2 -- processes > 1 always crashed with Fred/ROCS. ParallelEvaluator called pool.map(sampler.evaluate, ...). That pickles the bound method, hence the sampler, hence the evaluator, hence the SWIG-wrapped OEDock it holds -- which raises TypeError: cannot pickle 'SwigPyObject' object. The run died on the first batch, before any docking. Workers now construct their own evaluator instead of receiving one. The pool is created with an initializer that builds the evaluator inside each worker from its picklable Pydantic config, once per worker rather than once per molecule, so the OpenEye object never crosses the pipe. ThompsonSampler gains __getstate__/__setstate__ dropping the evaluator, pool and logger (the logger is rebuilt worker-side). Unlike a fork-inheritance workaround this is correct under spawn as well as fork, so it works on macOS and Windows too. Verified end to end with a real OEDock under spawn: the evaluator is confirmed unpicklable, and all 4 products still dock correctly across 2 workers. When no evaluator_config is available (a manually constructed sampler), the old ship-the-callable path is kept for picklable evaluators, and an unpicklable one now fails with an actionable message instead of a raw TypeError. Bug 3 -- segfault on import. evaluators.py imported OpenEye at module level, initialising the global libexpat parser. tqdm pulls in prompt_toolkit, whose formatter calls xml.dom.minidom.parseString() at module level and re-enters libexpat via pyexpat, conflicting with OpenEye's initialisation and killing the interpreter (exit 139) during import TACTICS. OpenEye is now imported lazily via _ensure_openeye() on first construction of an OpenEye evaluator. Also fixes joblib, which was imported inside the OpenEye try/except and so was undefined for MLClassifierEvaluator whenever OpenEye was absent. Adds 14 tests covering all three bugs, using an unpicklable stand-in so the SWIG path is exercised without an OpenEye licence. Bumps to 1.2.0 and adds a CHANGELOG. Reported-by: Donald van Pinxteren Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Documentation and notebooks that existed only in the working tree are now tracked. Added: - docs/handoffs/ -- session archives for 2026-03 through 2026-05 plus two research handoff documents, referenced from research/STATUS.md - research/blueprints/seh_caix_smarts.md -- recovered SMARTS recipes - research/STATUS.md -- current research state Notebook reorganization: the single manuscript_plots.py was split by scoring type and analysis, so it and two superseded comparison notebooks are removed in favour of manuscript_plots_docking.py, manuscript_plots_ROCS.py, manuscript_sar_plots.py, diagnostic_benchmark_plots.py and interactive_sar_explorer.py. Note: CLAUDE.md is gitignored (.gitignore:163) and stays local, so its code-map and parallel-evaluation updates are not part of this commit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The README had grown to 410 lines with duplicated structure -- two "Quick
Start" sections, two "Installation" sections, and both a "Package Structure"
and a "Repository Structure" tree -- so a newcomer could not quickly tell what
TACTICS does or how to start.
Restructured to a linear path: what it is -> whether it fits your problem ->
install -> a quickstart that runs -> parallelism for slow evaluators -> what
you can plug in -> where to go deeper. Advanced configuration is preserved in
collapsible sections rather than dropped, and the detailed API reference is
left to the Sphinx docs instead of duplicated here. 410 -> 229 lines.
Also corrects content that was wrong, not merely verbose:
- The parallel-processing example called get_preset("parallel_batch", ...),
a preset that does not exist and would raise. It now shows the real
config.processes field added in 1.2.0 (and that example could not have
worked before regardless, because from_config ignored processes).
- The tutorial table listed two notebooks that have been removed, and "Key
Features" claimed 5 presets when there are 4.
- Documents that Lookup/DB evaluators leave SMILES as "FAIL" because molecule
generation is skipped, and that macOS/Windows need an
if __name__ == "__main__" guard under the spawn start method.
Every API name in the README was checked against the package, and the
quickstart was executed end to end.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
Fixes the three bugs from the 2026-06-23 report by Donald van Pinxteren, which together made
processes > 1non-functional with Fred docking. Also lands outstanding local research work, simplifies the README, and cleans up accumulated generated artifacts.Bumps to v1.2.0 and adds a
CHANGELOG.md.The three cluster bugs
Bug 1 — parallelism could not be enabled.
from_config()hardcodedprocesses=1andThompsonSamplingConfighad no field for it, so a 1000-iteration docking screen ran on one core of a 128-core allocation with no error or warning. Addedprocesses/min_cpds_per_coreto the config and wired them through.Bug 2 —
processes > 1always crashed with Fred/ROCS.pool.map(sampler.evaluate, ...)pickled the bound method → the sampler → the evaluator → the SWIG-wrappedOEDock, raisingTypeError: cannot pickle 'SwigPyObject' objecton the first batch.Workers now build their own evaluator instead of receiving one. The pool is created with an
initializerthat constructs the evaluator inside each worker from its picklable Pydantic config — once per worker, not once per molecule — so the OpenEye object never crosses the pipe.ThompsonSamplergained__getstate__/__setstate__dropping the evaluator, pool and logger.Unlike a fork-inheritance workaround, this is correct under
spawnas well asfork, so it works on macOS/Windows too. No OpenEye code is modified — only the picklable config crosses the pipe.Bug 3 — segfault on import. OpenEye was imported at module scope, initialising global
libexpatstate;tqdm→prompt_toolkitthen re-entered expat viapyexpatat module scope, killing the interpreter (exit 139) duringimport TACTICS. OpenEye is now imported lazily on first construction of an OpenEye evaluator.Also fixes
joblib, which was imported inside the OpenEyetry/exceptand so was undefined forMLClassifierEvaluatoron any machine without OpenEye.Verification
OEDock, not a mock. Confirmed the evaluator is still unpicklable (the exact reportedTypeError), then ran real Fred docking withprocesses=2underspawn— the harder start method than the cluster'sfork. 4/4 products docked across 2 workers.main(SHA over the full evaluated-product sequence).processesdefaults to 1 and the sequential path is untouched.Also included
default_scoreforLookupEvaluator— sparse libraries (DEL read counts) where an absent combination is a true non-binder, not missing data. Defaults toNone, preserving existing NaN behaviour.max(z)beforeexp(); shift-invariant, so probabilities are unchanged, but overflow becomes impossible.RouletteWheelSelection.min_observationsis now inert but still accepted; both docstrings updated rather than left describing a gate that no longer exists.get_preset("parallel_batch", ...)preset that does not exist and would raise, listed two removed tutorials, and claimed 5 presets when there are 4.jobs/,plots/,*.bak, and OpenEye run artifacts are now gitignored (CLAUDE.mdalready claimedjobs/was ignored, but no rule existed); scratch files removed.docs/handoffs/,research/blueprints/,research/STATUS.md.Notes for review
spawnlocally, but not at the reporter's scale (1000 iterations, 128 cores, ~5.96M compounds). A short real-cluster run is the last check before telling users it is fixed.ForkAwareParallelEvaluatorand thepyexpat/xml.dom.minidompre-import shim.CLAUDE.mdis gitignored (.gitignore:163), so its code-map and parallel-evaluation updates are local only and not in this PR.ThompsonSamplingConfig.seeddoes not fully control run-to-run variation — the defaultEnhancedWarmupuses unseeded global RNGs. Deliberately not changed: TS is intrinsically stochastic and the 20-replicate design characterises that variation, so published results are unaffected.get_preset("baseline"/"legacy_rws", ..., batch_size=N)raisesTypeErrordespite the docstring advertisingbatch_size.🤖 Generated with Claude Code