public-api: rename, reshape, and tighten the cross-language surface#43
Open
schroedk wants to merge 14 commits into
Open
public-api: rename, reshape, and tighten the cross-language surface#43schroedk wants to merge 14 commits into
schroedk wants to merge 14 commits into
Conversation
…exports - Rename SparseMatrix to CsrMatrix and rename sparse_matrix.rs to csr_matrix.rs to match the existing module-naming convention. - Move LocalSolveError out of the crate-root re-exports; expose it via schwarz_precond::error::LocalSolveError. Make the error module pub. - Fix the redundant-link rustdoc warning on LocalSolver::solve_local. - Update within's internal callers (schur_complement, schwarz, local_solver, operator/tests) for the rename + relocated LocalSolveError.
Cross-language rename so Rust and Python use the same names:
- SolverParams → LsmrConfig (Python LSMR → LsmrConfig)
- Preconditioner config enum → PreconditionerConfig (Python likewise)
- FePreconditioner (built handle) → Preconditioner
- SchurComplement → LocalSolverConfig
Reshape PreconditionerConfig:
- Add explicit Off variant; Additive(LocalSolverConfig, ReductionStrategy)
collapses into Additive(AdditiveSchwarz) — a new struct that owns the
local solver + reduction strategy together.
- Option<&PreconditionerConfig> now means: None = library default,
Some(::Off) = explicit identity, Some(::Additive(_)) = tuned Schwarz.
- Drop solver_default(); the single LocalSolverConfig::default() uses
split_merge: Some(2) (no one-shot use case).
Make Preconditioner an opaque struct:
- Wraps a private inner enum. #[non_exhaustive] removed (future variants
are non-breaking additions to the inner enum). #[serde(transparent)]
keeps the postcard wire format identical to the inner enum.
- FeSchwarz is no longer reachable from the public surface.
Tighten module visibility on the within crate:
- domain / operator / orchestrate / solver are now pub(crate). Public
items remain re-exported from the crate root.
- Drop top-level Operator (use schwarz_precond::Operator),
DEFAULT_DENSE_SCHUR_THRESHOLD, Subdomain, and the
domain::{PartitionWeights, SubdomainCore} re-exports.
- DesignOperator stays reachable via a #[doc(hidden)] re-export for
integration tests and benches.
Close two silent-validation holes:
- Solver::solve rejects y.len() != n_obs with SolveError::InvalidInput
(was silent truncation by the zip with sqrt-weights).
- Python solve_batch rejects Y.shape[0] != n_obs up front (was bypassed
by empty Y).
Python API:
- Top-level within.* exposes the 9 stable call-site names; advanced
configs live in a new within.config submodule.
- Solver / solve / solve_batch accept a 4-form preconditioner argument:
None, PreconditionerConfig.{Off,Additive}, AdditiveSchwarz(...), or a
pre-built Preconditioner (reuse path).
- LocalSolverConfig is now a thin Python subclass over the PyO3 native
with __slots__ = (). approx_schur=None requests exact Schur; omitting
uses the library default approximate. Preserves the previous semantic
used in benchmarks/suites/fixest_comparison.py.
- Every PyO3 class reports __module__ == "within._within".
- All config and result classes are now picklable via __reduce__.
Added Rust convenience APIs:
- solve_with_preconditioner / solve_batch_with_preconditioner for
one-shot solves that reuse a pre-built Preconditioner without
holding a Solver.
- BatchSolveResult::n_dofs() / n_obs() accessors so callers can recover
design dimensions for empty batches.
- README.md (root): rewrite the Python quickstart for LsmrConfig +
PreconditionerConfig; the preconditioner section now documents the
4-form Union (None / PreconditionerConfig.{Off,Additive} /
AdditiveSchwarz / built Preconditioner). Rewrite the Rust API
section: opaque Preconditioner, two-channel signaling, within.config
module imports. Replace the "lower-level types" table with a module
visibility table reflecting the new pub(crate) boundary.
- crates/within/README.md: Rust example uses LsmrConfig +
PreconditionerConfig. Updated module description.
- python/within/README.md: solver.preconditioner is a property (was a
method); document the approx_schur=None = exact Schur semantic;
drop the removed FullSddm reference.
- CHANGELOG.md: consolidate the [Unreleased] block into the standard
Added/Changed/Removed sections with concise BREAKING bullets.
…itioner These were two-line forwarders to Solver::with_preconditioner(...).solve(...). The Python bridge now inlines that pattern directly; no released code depended on the wrappers (they only existed in the Unreleased CHANGELOG).
Replace the 9-arg pub(crate) BatchSolveResult::new and set_time_total setter with direct struct-literal construction and field assignment — both producer and only consumer live in the crate. Collapse the two adjacent pub use error::... lines (same names exported, one line).
- Drop dead Preconditioner::additive_reduction_strategy / resolved_additive_reduction_strategy accessors plus their FeSchwarz feeders (reduction_strategy, resolved_reduction_strategy, with_reduction_strategy). Zero external callers; only fed each other. Test rewrites build two preconditioners explicitly instead of using with_reduction_strategy. - Merge operator/preconditioner.rs into operator/schwarz.rs — the opaque Preconditioner handle wraps FeSchwarz and belongs next to it. - Hoist operator/gramian/cross_tab.rs to operator/cross_tab.rs and drop the 18-line operator/gramian.rs routing module; tests move with it. - Trim module-level rustdoc essays across lib.rs, config.rs, domain.rs, observation.rs, solver.rs, orchestrate.rs, operator.rs, operator/schwarz.rs, error.rs. lib.rs drops from 175 to 33 lines (problem formulation, normal equations, block structure, references essays gone; one-paragraph summary plus quick-start kept). Net: -489 lines, -1 file, -1 directory. No public API change. cargo test --workspace passes.
- BREAKING: BatchSolveResult fields are now pub; drop n_rhs/n_dofs/n_obs/
x_all/demeaned_all/converged/iterations/final_residual/time_solve/
time_total accessors. Keep x(i)/demeaned(i) slicing methods.
- within-py: inline extract_lsmr_config and extract_local_solver_{config,
or_default} into their single callers.
- within: drop compute_dense, DenseSchurResult, dense_from_elimination
and the two dense-Schur cross-check tests (all #[cfg(test)] scaffolding;
production uses only compute_dense_anchored).
- schwarz-precond, schur_complement: trim module-level rustdoc essays.
Drop math derivations, paper references, and forward-compat justification prose from eight module heads, reducing each to a short orientation that names the public symbols readers actually need. Inline the one-line dense_fast_path_enabled predicate at its sole caller. Public API, serialization, and behavior unchanged; cargo test --workspace stays green.
Deletions (all test-only or single-use): - find_active_levels and CrossTab::build_for_pair (cross_tab.rs): both #[cfg(test)] and consumed only by cross_tab/tests.rs — tests now use the production find_all_active_levels + build_for_pair_with_active path that domain/factor_pairs.rs::build_local_domains already uses. - sample_factor_levels (observation.rs): single-call helper used only by the in-file test; inlined the vec! literal. - BlockElimSolver::uses_dense_reduced_factor (local_solver.rs): bumped the reduced_factor field to pub(crate) and inlined the matches! check at the three test sites that asserted on it. Inlines: - lsmr_stop_reason at its single call site in lsmr_from_bidiag. - validate_lsmr_preconditioner at its single call site in mlsmr.
…eSchwarz struct
The AdditiveSchwarz wrapper struct held only (local_solver, reduction)
and was always reached through PreconditionerConfig::Additive(_) at
every Rust call site — there was no reuse-as-a-value benefit. Inline
the fields directly on the enum variant:
Additive { local_solver: LocalSolverConfig, reduction: ReductionStrategy }
Saves one nesting level at the three Rust construction sites
(within-py boundary, orchestrate_solve test, fixest bench) and removes
the standalone Rust struct. The PyAdditiveSchwarz PyO3 class is a
separate type and is unaffected, so Python users still write
solve(..., preconditioner=AdditiveSchwarz(...)) as before.
Also drops the corresponding entry from the Unreleased CHANGELOG since
the public Rust struct never shipped.
Collapse the four separate LSMR-related bullets in [Unreleased] Added into a single **Modified LSMR:** entry to match the themed-bullet style used in [0.1.0]. No information dropped — every detail (local_size, LsmrStopReason, non-finite rejection) survives inline.
Reshape Solver so it owns problem state (design, weights, preconditioner) and accepts LSMR tuning per call: Solver::new(design, weights, precond) // problem state only solver.solve(&y, &lsmr_opts) // tuning per call solver.solve_batch(&ys, &lsmr_opts) // tuning per call The free `solve` / `solve_batch` keep `&LsmrOptions` in the same slot. On the Python side, `Solver(...)` loses its `config=` kwarg; `solver.solve(y, options=...)` and `solver.solve_batch(Y, options=...)` accept it instead. Free functions rename `config=` to `options=`. Other changes folded in: - Rename LsmrConfig -> LsmrOptions (Rust + Python). End users see SolverParams -> LsmrOptions; the intermediate name doesn't ship. - Add `From<&Preconditioner> for PreconditionerInput` so `&precond` works as the fifth Solver::new shape. Cheap clone is preserved via refcount bumps on the inner Arc'd subdomains + BufferPool. - Add `BuildError::PreconditionerDimensionMismatch` -- Solver::new fails fast when a reused preconditioner's nrows/ncols don't match design.n_dofs instead of bubbling a confusing late mlsmr error. - Document the cheap-clone invariant on Preconditioner, including the BufferPool interior-mutability nuance and the requirement for future Variant additions. - Alias `schwarz_precond::lsmr` to `lsmr_solve` in solver.rs so the `lsmr` identifier is free for the new per-call parameter. - Fix Python Solver.solve_batch: rename PyO3 arg `y_matrix` -> `Y` (was a stub/impl mismatch) and validate Y.shape[0] against the solver's n_obs up front (parity with the free solve_batch). - Rename test_solver_from_design -> test_solver_accepts_prebuilt_design (constructor was inlined into Solver::new earlier on this branch).
Two new integration tests guard parts of the public surface that neither the unit tests nor the workspace build catch on their own: - api_pinning.rs: compile-time pinning of the six preconditioner-arg shapes accepted by Solver::new (bare None, &PreconditionerConfig, Some(&PreconditionerConfig), owned PreconditionerConfig, owned Preconditioner, &Preconditioner). If a future PR adds a From impl that makes bare `None` ambiguous, or removes one of these shapes, this file fails to compile -- an explicit signal. - wire_format_fixture.rs + fixtures/preconditioner_v1.postcard: load a known postcard payload via include_bytes!, deserialize into a Preconditioner, plug it into Solver::new, run a solve, and compare coefficients against a fresh-default solver. Defends the CHANGELOG's `#[serde(transparent)]` wire-stability claim against silent encoding drift (which the existing same-build round-trip test cannot detect). A companion `#[ignore]`'d regenerate_wire_format_fixture test writes a fresh fixture for intentional wire-format bumps.
Codex-flagged inconsistencies and missed entries from the LsmrOptions move: - Root README Python API table: `config?` -> `options?` for the free `solve` / `solve_batch` functions; stale `Solver::with_preconditioner` reference -> `Solver::new(.., precond)`. - Root README Rust example: remove dead references to the standalone Rust `AdditiveSchwarz` struct (collapsed in aefd08b); use the struct-variant syntax `PreconditionerConfig::Additive { local_solver, reduction }`. Drop `AdditiveSchwarz` from the type tables. - Within/Python READMEs: `.solve(y)` -> `.solve(y, options?)` to match the new method signature. - CHANGELOG (Added): new From<&Preconditioner> impl; new PreconditionerDimensionMismatch BuildError variant. - CHANGELOG (Changed): `Solver::new` shape change — `impl IntoDesign` + `impl Into<PreconditionerInput>` with the legacy `from_design` / `from_design_with_preconditioner` / `with_preconditioner` constructors removed; LsmrOptions move from constructor to call site. - CHANGELOG (Fixed): new section for the Python solve_batch fixes (Y kwarg actually accepted; Y.shape[0] validated up front). - CHANGELOG: soften the `LsmrStopReason` claim — it's carried on schwarz_precond::LsmrResult, not yet surfaced on within::SolveResult.
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
Cleanup pass on the public API of
within(Rust + Python) andschwarz-precond. Aligns Rust/Python names, makes internal modules honestly private, closes silent-validation holes, separates problem state from solve-time tuning, and trims dead scaffolding. Closes #33.Public API
SolverParams→LsmrOptions,Preconditioner(config enum)→PreconditionerConfig,FePreconditioner→Preconditioner,SchurComplement→LocalSolverConfig,SparseMatrix→CsrMatrix.PreconditionerConfig: newOffvariant;Additive { local_solver, reduction }(struct variant; the wrapping RustAdditiveSchwarzstruct was collapsed).Preconditioner: opaque public struct;#[serde(transparent)]pins the wire format. Cheap O(1) clone via internalArc.Solver::new:impl IntoDesign+impl Into<PreconditionerInput>; accepts five preconditioner shapes (None,&Cfg,Some(&Cfg), ownedCfg, owned-or-&Preconditioner). Removesfrom_design,from_design_with_preconditioner,with_preconditioner.LsmrOptionsmoves fromSolver::newtoSolver::solve/solve_batch. Python:Solver(...)losesconfig=;solver.solve(y, options=...)andsolver.solve_batch(Y, options=...)accept it instead. Free functions:config=→options=.Designis pure data + layout; weights live onDesignOperator::new/Solver::new/build_preconditioner.Validation
Solver::solverejectsy.len() != n_obs(was silent truncation).Solver::newrejects a prebuiltPreconditionerwhosenrows/ncolsdon't matchdesign.n_dofsvia newBuildError::PreconditionerDimensionMismatch.Solver.solve_batchrejectsY.shape[0] != n_obsup front (incl. empty-Ycase).Operator::apply/apply_adjointreturnResult<(), SolveError>.Tests
api_pinning.rscompile-pins the fiveSolver::newpreconditioner shapes.wire_format_fixture.rs+ 218-byte fixture: defends the#[serde(transparent)]wire-stability claim against silent encoding drift.Module visibility
within::{domain, operator, orchestrate, solver}arepub(crate); public items remain re-exported from the crate root. Newwithin.configPython submodule exposes advanced configs.Test plan
cargo test --workspacepixi run test— 97 Python testscargo doc -p within -p schwarz-precond --no-depsclean underRUSTDOCFLAGS='-D warnings'