Refactor steering code#213
Merged
Merged
Conversation
…noiser Split the monolithic steering.py into a steering/ package with: - collective_variables.py: CollectiveVariable base, CaCaDistance, PairwiseClash - potentials.py: Potential base, UmbrellaPotential - utils.py: shared utilities (resampling, x0 prediction, reward computation) - dpm_smc.py: SMC (Sequential Monte Carlo) steered denoiser Also refactors denoiser.py with DPM-Solver++ helper functions and updates sample.py to use the Hydra denoiser pattern. This is the first half of the steering work from PR #203. FKC denoiser, structure-reference CVs (RMSD, FractionNativeContacts), LinearPotential, and example notebooks will follow in a separate PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Neither file is referenced in code. bioemu.yaml was a Hydra root config but no @hydra.main or hydra.compose ever uses it. stochastic_dpm.yaml was only referenced by bioemu.yaml. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Parametrize TestUmbrellaPotentialLossFn (5 methods → 1 parametrized + 2) - Remove redundant test_two_equal_groups (duplicate of test_uniform_weights) - Tighten loose thresholds: ESS tolerance 0.01→1e-5, 0.05→0.005, energy tolerance 1e-4→1e-6, stratified resample N-5→N-2 - Strengthen smoke tests: add gradient magnitude checks for CaCaDistance and antisymmetric projection, use clashing positions for PairwiseClash - Verify unsteered log_weights are exactly zero (assert_close vs allclose) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Move all imports to top of each file - Consolidate TestCaCaDistance into single test (shape + values + grad) - Merge test_no_clash + test_offset_respects_separation into one test - Move ESS and rotation gradient tests from test_denoisers.py to test_utils.py (they test utils functions, not denoiser); delete test_denoisers.py - Remove TestPhysicalSteeringConfig (trivial YAML-loading tests) - Move TestPotentialForwardBackward to test_potentials.py (where it belongs) - Extract module-level fixtures (sdes, batch, score_model) in test_integration - Add TestSmcDpmEquivalence: unsteered SMC vs DPM solver comparison, and steered SMC with ess_threshold=0 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Move all inline imports to module top in test_integration.py - TestSmcDpmEquivalence: use noise=0 for deterministic comparison (atol=1e-4 vs previous rel_diff<0.5); add test that steered with ess_threshold=0 produces same particle set as unsteered - TestResampleBasedOnLogWeights: consolidate 4 tests into parametrized fixture-based test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors BioEmu’s steering functionality into a dedicated bioemu.steering/ package, keeping the SMC (Sequential Monte Carlo) denoiser and related components modular and reusable, while updating sampling/config/docs/tests to use a single Hydra denoiser config for steered runs.
Changes:
- Extract steering primitives into
src/bioemu/steering/(CVs, potentials, resampling/reward utilities, SMC DPM-Solver). - Simplify sampling and configs so steering is provided via a single
denoiser_configYAML (no separatesteering_configCLI/API argument). - Replace the old monolithic steering test with a split test suite (unit + lightweight integration + chignolin e2e).
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_steering.py | Removes legacy monolithic steering test file. |
| tests/steering/init.py | Adds steering test package marker. |
| tests/steering/test_utils.py | Adds unit tests for resampling/ESS/x0 helpers. |
| tests/steering/test_potentials.py | Adds unit tests for UmbrellaPotential and replacements for break/clash behavior. |
| tests/steering/test_collective_variables.py | Adds unit tests for CaCaDistance and PairwiseClash. |
| tests/steering/test_integration.py | Adds lightweight integration tests for SMC wiring and generate_batch pipeline via mocks. |
| tests/steering/test_chignolin_e2e.py | Adds end-to-end steering tests that run full sample() pipeline. |
| src/bioemu/steering/init.py | Defines steering public API re-exports (CVs, potentials, utils, SMC). |
| src/bioemu/steering/collective_variables.py | Introduces CV base class + CaCaDistance and PairwiseClash. |
| src/bioemu/steering/potentials.py | Introduces potential base class + UmbrellaPotential. |
| src/bioemu/steering/utils.py | Adds steering utilities: config validation, resampling, ESS, reward/grad plumbing, x0/R0 prediction. |
| src/bioemu/steering/dpm_smc.py | Adds SMC DPM-Solver denoiser loop and SMC step implementation. |
| src/bioemu/steering.py | Deletes old monolithic steering module. |
| src/bioemu/sample.py | Removes separate steering config plumbing; denoiser_config now fully configures steering via Hydra. |
| src/bioemu/denoiser.py | Extracts DPM-Solver helper primitives used by both unsteered DPM and SMC solver. |
| src/bioemu/config/steering/physical_steering.yaml | Converts steering config into a self-contained Hydra denoiser config targeting dpm_solver_smc. |
| src/bioemu/config/denoiser/stochastic_dpm.yaml | Removes unused/superseded stochastic DPM config. |
| src/bioemu/config/bioemu.yaml | Removes unused top-level config file. |
| README.md | Updates steering docs/CLI examples to use a single steering YAML passed as --denoiser_config. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ludwigwinkler
previously approved these changes
Apr 27, 2026
…nsolidation - Add value range/type checks in validate_steering_config (num_particles, ess_threshold, start/end) - Fix type hints: ChemGraph -> BatchType on resample_based_on_log_weights - Convert resampling indices to CPU before list indexing (GPU perf fix) - Use .reshape() instead of .view() for non-contiguous tensor safety - Fix mutable default set() in dpm_solver_smc signature - Remove unused ChemGraph import (fixes ruff F401) - Consolidate e2e tests with pytest.mark.parametrize Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
SummarySummary
Coveragesrc.bioemu - 88.9%
src.bioemu.colabfold_inline - 79.1%
src.bioemu.hpacker_setup - 69.2%
src.bioemu.steering - 89.8%
src.bioemu.training - 100%
|
ludwigwinkler
approved these changes
May 4, 2026
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.
Refactor: extract steering into modular package with SMC denoiser
Summary
Splits the monolithic steering.py into a well-organized steering/ package, keeping only the SMC (Sequential Monte Carlo) denoiser. This is the first of two PRs to split #203 — FKC-specific code will follow in a separate PR.
Motivation
The original steering.py (358 lines) mixed collective variables, potentials, utility functions. To make a general so we can include both SMC and FKC, this refactor:
Changes
Bumped up version to 1.4.0
Package structure — src/bioemu/steering.py → src/bioemu/steering/:
denoiser.py cleanup:
_predict_midpoint, second_order_step_dpmsolver_plusplus) used by both
dpm_solver and dpm_solver_smc
as clean abstractions
sample.py cleanup:
Config cleanup:
What's NOT in this PR