Skip to content

Drop AdditiveSchwarzDiagnostics from public API#38

Merged
schroedk merged 1 commit into
mainfrom
drop-additive-schwarz-diagnostics
May 11, 2026
Merged

Drop AdditiveSchwarzDiagnostics from public API#38
schroedk merged 1 commit into
mainfrom
drop-additive-schwarz-diagnostics

Conversation

@schroedk
Copy link
Copy Markdown
Collaborator

Summary

  • Removes AdditiveSchwarzDiagnostics (Rust struct, pub use re-exports, Python class, additive_schwarz_diagnostics accessors at every layer).
  • Inlines the 5 scheduling metrics directly into AdditiveScheduler — they remain private to the Auto heuristic.
  • Keeps ReductionStrategy and the Preconditioner::Additive(_, ReductionStrategy) knob unchanged: the wall-clock-vs-memory tradeoff (12x peak memory at scale) is a real user concern.

Closes #34.

Why

Empirical bake-off across small / medium / huge regimes (see the issue comments) showed:

  • Auto's wall-clock decisions are within the ~5% measurement noise floor on every regime in the project's normal operating range.
  • AdditiveSchwarzDiagnostics had zero readers outside the scheduler itself — no test asserted on its fields, no benchmark inspected it, no production caller read it.
  • The heuristic's tuning constants are private, so an external caller could not reproduce or override decisions from the diagnostics anyway.

ReductionStrategy is kept because memory matters: on a 100K-DOF problem, ParallelReduction peaks at ~9.6 MB vs AtomicScatter's ~800 KB (12x). At 1M DOFs that becomes ~96 MB vs ~8 MB. Memory-constrained or server-scenario users need the override.

Test plan

  • cargo build --workspace --release clean
  • cargo test --workspace --exclude within-py all pass (within-py lib-test failure is the expected "PyO3 can't run standalone" issue per CLAUDE.md)
  • pixi run develop rebuilds the extension cleanly
  • pytest tests/ 97/97 pass
  • Within nested-rayon regression test rewritten to use subdomains().len() and direct inner_parallelism_work_estimate() iteration — same assertions, no diagnostics() dependency

Empirical bake-off (see issue) showed Auto's wall-clock decisions are
within noise of optimal across the project's relevant range, and the
diagnostics struct had zero readers outside the scheduler itself.
Inline the metrics into AdditiveScheduler and remove the public surface.

ReductionStrategy is kept: AtomicScatter vs ParallelReduction is a
real memory-vs-time tradeoff (12x peak memory at scale) that users
may want to control.
@schroedk schroedk merged commit 798c342 into main May 11, 2026
3 checks passed
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.

Investigate removing AdditiveSchwarzDiagnostics and ReductionStrategy

1 participant