⚙️ Core: Dependency Inversion for Ising Simulation #364
Conversation
- Extracted `MetropolisSolver<R>` from `SpinLattice` to separate simulation logic from state. - Injected `R: Rng` into `MetropolisSolver` to enable deterministic testing. - Added `SpinLattice::new_with_rng` constructor. - Deprecated `SpinLattice::metropolis_step` and `evolve` in favor of the solver approach. - Added `test_ising_deterministic` to verify reproducibility. Violations addressed: SRP (State vs Logic), DIP (Hardcoded RNG). Debt Reduction: Enabled deterministic testing for stochastic simulation. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
This PR refactors the Ising Model simulation (
math_explorer/src/physics/stat_mech/ising.rs) to adhere to the "Systems Core" principles of Dependency Inversion and Single Responsibility.The Violation:
The
SpinLatticestruct previously hardcodedrand::thread_rng()in its methods, making it impossible to seed the simulation for deterministic testing. It also mixed the concerns of data storage (the grid) with the simulation algorithm (Metropolis-Hastings).The Fix:
MetropolisSolver<R>struct that holds the RNG and implements the simulation steps (stepandevolve).R: Rng, allowingStdRng(seeded) orThreadRngto be used.SpinLatticeare kept but marked#[deprecated]and delegate to the new solver to ensure backward compatibility.Verification:
test_ising_deterministicwhich proves that two simulations with the same seed produce bit-identical results.PR created automatically by Jules for task 4115380246471816483 started by @fderuiter