Skip to content

Conversation

@fderuiter
Copy link
Owner

Implemented batched Metropolis updates for the Ising model with lookup table optimization.


PR created automatically by Jules for task 835588068557748167 started by @fderuiter

📉 The Bottleneck:
The `SpinLattice::metropolis_step` method was limited by `rand::thread_rng()` initialization overhead and expensive `exp()` calculations in the hot loop (13.9 M/s).

🚀 The Boost:
Before: ~0.72s (13.9 M/s)
After: ~0.39s (25.7 M/s)
Speedup: 1.85x

💻 Technical Detail:
- Implemented `SpinLattice::evolve` for batched updates.
- Added a stack-allocated lookup table to precompute Boltzmann factors (`exp(-beta * dE)`) for all 10 possible local configurations.
- Reused a single RNG instance across the batch to eliminate TLS overhead.
- Added `examples/bench_ising_custom.rs` to verify the speedup.

🧪 Verification:
- `cargo test` passed.
- Benchmark confirmed 1.85x improvement.

Co-authored-by: fderuiter <127706008+fderuiter@users.noreply.github.com>
@google-labs-jules
Copy link
Contributor

👋 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 @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@fderuiter
Copy link
Owner Author

The failing job encountered this error early in its workflow:
"'toolchain' is a required input"

This occurs because the job expects a toolchain input variable to be set, but it is missing or empty. In your workflow file (.github/workflows/ci.yml), the following section sets up the Rust toolchain:

- name: Set up Rust
  uses: dtolnay/rust-toolchain@stable
  with:
    toolchain: stable
    components: clippy, rustfmt

The problem may arise if the input variable toolchain is not properly defined, or if there's an override or missing environment variable in the workflow or an Action that expects this input.

How to fix:

  • Ensure the toolchain input is always provided. The current configuration appears correct, but check any recent changes to this or upstream actions.
  • If using matrix jobs or dynamic values, ensure toolchain is always set, e.g.
with:
  toolchain: ${{ matrix.toolchain || 'stable' }}
  components: clippy, rustfmt

If a job or a step expects $toolchain defined elsewhere (custom scripts, composite actions), verify it is present.

Quick code suggestion:
Double-check lines 25 and the context where $toolchain is referenced. If you use custom scripts or composite actions relying on $toolchain, make sure you pass it in the workflow:

env:
  toolchain: stable

Add this to your job or step environment if needed.

Summary:
Define or pass the required toolchain input to fix the error. If you need finer control, set it in the env block or ensure any referenced step or script receives it.

@fderuiter
Copy link
Owner Author

Run cargo fmt -- --check
Diff in /home/runner/work/math/math/math_explorer/examples/bench_ising_custom.rs:1:
-use math_explorer::physics::stat_mech::ising::SpinLattice;
use math_explorer::physics::stat_mech::KB;
+use math_explorer::physics::stat_mech::ising::SpinLattice;
use std::time::Instant;

fn main() {
Diff in /home/runner/work/math/math/math_explorer/examples/bench_ising_custom.rs:12:

 let iterations = 10_000_000;
  • println!("Benchmarking Ising Model {}x{} for {} iterations...", width, height, iterations);
  • println!(

  •    "Benchmarking Ising Model {}x{} for {} iterations...",
    
  •    width, height, iterations
    
  • );

    // --- Before: metropolis_step loop ---
    let mut lattice = SpinLattice::new(width, height);
    Diff in /home/runner/work/math/math/math_explorer/examples/bench_ising_custom.rs:22:
    }
    let duration = start.elapsed();
    let old_speed = (iterations as f64 / duration.as_secs_f64()) / 1_000_000.0;

  • println!("Before (metropolis_step): {:.4}s, {:.2} M/s", duration.as_secs_f64(), old_speed);
  • println!(

  •    "Before (metropolis_step): {:.4}s, {:.2} M/s",
    
  •    duration.as_secs_f64(),
    
  •    old_speed
    
  • );

    // --- After: evolve batch ---
    let mut lattice = SpinLattice::new(width, height);
    Diff in /home/runner/work/math/math/math_explorer/examples/bench_ising_custom.rs:30:
    lattice.evolve(iterations, temp, j_coupling, h_field);
    let duration = start.elapsed();
    let new_speed = (iterations as f64 / duration.as_secs_f64()) / 1_000_000.0;

  • println!("After (evolve): {:.4}s, {:.2} M/s", duration.as_secs_f64(), new_speed);
  • println!(

  •    "After (evolve):           {:.4}s, {:.2} M/s",
    
  •    duration.as_secs_f64(),
    
  •    new_speed
    
  • );

    let speedup = new_speed / old_speed;
    println!("Speedup: {:.2}x", speedup);
    Error: Process completed with exit code 1.

@google-labs-jules
Copy link
Contributor

The failing job encountered this error early in its workflow:
"'toolchain' is a required input"

This occurs because the job expects a toolchain input variable to be set, but it is missing or empty. In your workflow file (.github/workflows/ci.yml), the following section sets up the Rust toolchain:

- name: Set up Rust
  uses: dtolnay/rust-toolchain@stable
  with:
    toolchain: stable
    components: clippy, rustfmt

The problem may arise if the input variable toolchain is not properly defined, or if there's an override or missing environment variable in the workflow or an Action that expects this input.

How to fix:

  • Ensure the toolchain input is always provided. The current configuration appears correct, but check any recent changes to this or upstream actions.
  • If using matrix jobs or dynamic values, ensure toolchain is always set, e.g.
with:
  toolchain: ${{ matrix.toolchain || 'stable' }}
  components: clippy, rustfmt

If a job or a step expects $toolchain defined elsewhere (custom scripts, composite actions), verify it is present.

Quick code suggestion:
Double-check lines 25 and the context where $toolchain is referenced. If you use custom scripts or composite actions relying on $toolchain, make sure you pass it in the workflow:

env:
  toolchain: stable

Add this to your job or step environment if needed.

Summary:
Define or pass the required toolchain input to fix the error. If you need finer control, set it in the env block or ensure any referenced step or script receives it.

I have added toolchain: stable to the global environment variables in .github/workflows/ci.yml as suggested to ensure the toolchain input is properly defined.

Co-authored-by: fderuiter <127706008+fderuiter@users.noreply.github.com>
@fderuiter
Copy link
Owner Author

Run cargo fmt -- --check
Diff in /home/runner/work/math/math/math_explorer/examples/bench_ising_custom.rs:1:
-use math_explorer::physics::stat_mech::ising::SpinLattice;
use math_explorer::physics::stat_mech::KB;
+use math_explorer::physics::stat_mech::ising::SpinLattice;
use std::time::Instant;

fn main() {
Diff in /home/runner/work/math/math/math_explorer/examples/bench_ising_custom.rs:12:

 let iterations = 10_000_000;
  • println!("Benchmarking Ising Model {}x{} for {} iterations...", width, height, iterations);
  • println!(

  •    "Benchmarking Ising Model {}x{} for {} iterations...",
    
  •    width, height, iterations
    
  • );

    // --- Before: metropolis_step loop ---
    let mut lattice = SpinLattice::new(width, height);
    Diff in /home/runner/work/math/math/math_explorer/examples/bench_ising_custom.rs:22:
    }
    let duration = start.elapsed();
    let old_speed = (iterations as f64 / duration.as_secs_f64()) / 1_000_000.0;

  • println!("Before (metropolis_step): {:.4}s, {:.2} M/s", duration.as_secs_f64(), old_speed);
  • println!(

  •    "Before (metropolis_step): {:.4}s, {:.2} M/s",
    
  •    duration.as_secs_f64(),
    
  •    old_speed
    
  • );

    // --- After: evolve batch ---
    let mut lattice = SpinLattice::new(width, height);
    Diff in /home/runner/work/math/math/math_explorer/examples/bench_ising_custom.rs:30:
    lattice.evolve(iterations, temp, j_coupling, h_field);
    let duration = start.elapsed();
    let new_speed = (iterations as f64 / duration.as_secs_f64()) / 1_000_000.0;

  • println!("After (evolve): {:.4}s, {:.2} M/s", duration.as_secs_f64(), new_speed);
  • println!(

  •    "After (evolve):           {:.4}s, {:.2} M/s",
    
  •    duration.as_secs_f64(),
    
  •    new_speed
    
  • );

    let speedup = new_speed / old_speed;
    println!("Speedup: {:.2}x", speedup);
    Error: Process completed with exit code 1.

@google-labs-jules
Copy link
Contributor

Run cargo fmt -- --check
Diff in /home/runner/work/math/math/math_explorer/examples/bench_ising_custom.rs:1:
-use math_explorer::physics::stat_mech::ising::SpinLattice;
use math_explorer::physics::stat_mech::KB;
+use math_explorer::physics::stat_mech::ising::SpinLattice;
use std::time::Instant;

fn main() {
Diff in /home/runner/work/math/math/math_explorer/examples/bench_ising_custom.rs:12:

 let iterations = 10_000_000;
  • println!("Benchmarking Ising Model {}x{} for {} iterations...", width, height, iterations);
  • println!(

  •    "Benchmarking Ising Model {}x{} for {} iterations...",
    
  •    width, height, iterations
    
  • );

    // --- Before: metropolis_step loop ---
    let mut lattice = SpinLattice::new(width, height);
    Diff in /home/runner/work/math/math/math_explorer/examples/bench_ising_custom.rs:22:
    }
    let duration = start.elapsed();
    let old_speed = (iterations as f64 / duration.as_secs_f64()) / 1_000_000.0;

  • println!("Before (metropolis_step): {:.4}s, {:.2} M/s", duration.as_secs_f64(), old_speed);
  • println!(

  •    "Before (metropolis_step): {:.4}s, {:.2} M/s",
    
  •    duration.as_secs_f64(),
    
  •    old_speed
    
  • );

    // --- After: evolve batch ---
    let mut lattice = SpinLattice::new(width, height);
    Diff in /home/runner/work/math/math/math_explorer/examples/bench_ising_custom.rs:30:
    lattice.evolve(iterations, temp, j_coupling, h_field);
    let duration = start.elapsed();
    let new_speed = (iterations as f64 / duration.as_secs_f64()) / 1_000_000.0;

  • println!("After (evolve): {:.4}s, {:.2} M/s", duration.as_secs_f64(), new_speed);
  • println!(

  •    "After (evolve):           {:.4}s, {:.2} M/s",
    
  •    duration.as_secs_f64(),
    
  •    new_speed
    
  • );

    let speedup = new_speed / old_speed;
    println!("Speedup: {:.2}x", speedup);
    Error: Process completed with exit code 1.

I have run cargo fmt to fix the formatting issues in math_explorer/examples/bench_ising_custom.rs.

Co-authored-by: fderuiter <127706008+fderuiter@users.noreply.github.com>
@fderuiter
Copy link
Owner Author

Run cargo clippy -- -D warnings
Checking libm v0.2.15
Checking libc v0.2.178
Checking bytemuck v1.24.0
Checking cfg-if v1.0.4
Checking zerocopy v0.8.31
Checking safe_arch v0.7.4
Checking wide v0.7.33
Checking num-traits v0.2.19
Checking getrandom v0.2.16
Checking rand_core v0.6.4
Checking rawpointer v0.2.1
Checking matrixmultiply v0.3.10
Checking typenum v1.19.0
Checking num-integer v0.1.46
Checking num-complex v0.4.6
Checking ppv-lite86 v0.2.21
Checking approx v0.5.1
Checking num-rational v0.4.2
Checking az v1.2.1
Checking simba v0.8.1
Checking rand_chacha v0.3.1
Checking rand v0.8.5
Checking gmp-mpfr-sys v1.6.8
Checking equivalent v1.0.2
Checking hashbrown v0.16.1
Checking strength_reduce v0.2.4
Checking rand_distr v0.4.3
Checking transpose v0.2.3
Checking rug v1.28.0
Checking indexmap v2.12.1
Checking simba v0.9.1
Checking primal-check v0.3.4
Checking fixedbitset v0.4.2
Checking petgraph v0.6.5
Checking rustfft v6.4.1
Checking nalgebra v0.32.6
Checking nalgebra v0.34.1
Checking wigner-symbols v0.5.0
Checking quadrature v0.1.2
Checking statrs v0.17.1
Checking math_explorer v0.1.0 (/home/runner/work/math/math/math_explorer)
error: the loop variable s_idx is used to index lut
--> src/physics/stat_mech/ising.rs:177:22
|
177 | for s_idx in 0..2 {
| ^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.93.0/index.html#needless_range_loop
= note: -D clippy::needless-range-loop implied by -D warnings
= help: to override -D warnings add #[allow(clippy::needless_range_loop)]
help: consider using an iterator and enumerate()
|
177 - for s_idx in 0..2 {
177 + for (s_idx, ) in lut.iter_mut().enumerate() {
|

error: the loop variable sum_idx is used to index lut
--> src/physics/stat_mech/ising.rs:179:28
|
179 | for sum_idx in 0..5 {
| ^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.93.0/index.html#needless_range_loop
help: consider using an iterator and enumerate()
|
179 - for sum_idx in 0..5 {
179 + for (sum_idx, ) in lut.iter_mut().enumerate() {
|

error: could not compile math_explorer (lib) due to 2 previous errors
Error: Process completed with exit code 101.

@google-labs-jules
Copy link
Contributor

Run cargo clippy -- -D warnings
Checking libm v0.2.15
Checking libc v0.2.178
Checking bytemuck v1.24.0
Checking cfg-if v1.0.4
Checking zerocopy v0.8.31
Checking safe_arch v0.7.4
Checking wide v0.7.33
Checking num-traits v0.2.19
Checking getrandom v0.2.16
Checking rand_core v0.6.4
Checking rawpointer v0.2.1
Checking matrixmultiply v0.3.10
Checking typenum v1.19.0
Checking num-integer v0.1.46
Checking num-complex v0.4.6
Checking ppv-lite86 v0.2.21
Checking approx v0.5.1
Checking num-rational v0.4.2
Checking az v1.2.1
Checking simba v0.8.1
Checking rand_chacha v0.3.1
Checking rand v0.8.5
Checking gmp-mpfr-sys v1.6.8
Checking equivalent v1.0.2
Checking hashbrown v0.16.1
Checking strength_reduce v0.2.4
Checking rand_distr v0.4.3
Checking transpose v0.2.3
Checking rug v1.28.0
Checking indexmap v2.12.1
Checking simba v0.9.1
Checking primal-check v0.3.4
Checking fixedbitset v0.4.2
Checking petgraph v0.6.5
Checking rustfft v6.4.1
Checking nalgebra v0.32.6
Checking nalgebra v0.34.1
Checking wigner-symbols v0.5.0
Checking quadrature v0.1.2
Checking statrs v0.17.1
Checking math_explorer v0.1.0 (/home/runner/work/math/math/math_explorer)
error: the loop variable s_idx is used to index lut
--> src/physics/stat_mech/ising.rs:177:22
|
177 | for s_idx in 0..2 {
| ^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.93.0/index.html#needless_range_loop
= note: -D clippy::needless-range-loop implied by -D warnings
= help: to override -D warnings add #[allow(clippy::needless_range_loop)]
help: consider using an iterator and enumerate()
|
177 - for s_idx in 0..2 {
177 + for (s_idx, ) in lut.iter_mut().enumerate() {
|

error: the loop variable sum_idx is used to index lut
--> src/physics/stat_mech/ising.rs:179:28
|
179 | for sum_idx in 0..5 {
| ^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.93.0/index.html#needless_range_loop
help: consider using an iterator and enumerate()
|
179 - for sum_idx in 0..5 {
179 + for (sum_idx, ) in lut.iter_mut().enumerate() {
|

error: could not compile math_explorer (lib) due to 2 previous errors
Error: Process completed with exit code 101.

I have updated the loop to use iter_mut().enumerate() as suggested by Clippy to avoid the needless_range_loop error.

Co-authored-by: fderuiter <127706008+fderuiter@users.noreply.github.com>
@fderuiter fderuiter merged commit b33fb76 into main Jan 26, 2026
1 check passed
@fderuiter fderuiter deleted the profiler/ising-optimization-835588068557748167 branch January 26, 2026 23:45
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