schwarz-precond: drop LocalSolveInvoker generic#37
Merged
Conversation
The invoker trait existed to thread one bool ("may the local solver use
inner parallelism?") through three impls — a default, an FE-specific one,
and a test-only one — at the cost of an `I: LocalSolveInvoker` type
parameter on every preconditioner-using API.
Move the hint onto `LocalSolver::solve_local` directly. `SchwarzPreconditioner`
and `AdditiveExecutor` drop the `I` parameter. `FeLocalSolveInvoker` is gone;
`BlockElimSolver::solve_local_with_parallelism` folds into the trait method.
The nested-Rayon regression test now overrides `LocalSolver::solve_local`
on its own impl instead of carrying a custom invoker.
Net -107 LOC. All workspace tests, the Python suite, and the nested-Rayon
deadlock regression remain green.
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.
Closes #31.
Summary
LocalSolver::solve_localnow takesallow_inner_parallelism: booldirectly. Solvers with no nested-parallel region ignore the hint (_allow_inner_parallelism).SchwarzPreconditionerandAdditiveExecutorlose theI: LocalSolveInvokertype parameter.LocalSolveInvoker,DefaultLocalSolveInvoker,FeLocalSolveInvoker, andwith_strategy_and_invokerare gone.BlockElimSolver::solve_local_with_parallelismfolds into itsLocalSolver::solve_localimpl.FeSchwarzis nowSchwarzPreconditioner<BlockElimSolver>.ParallelReductionregression test overridesLocalSolver::solve_localon its own impl (no custom invoker needed).Net -107 LOC.
This is a breaking change to
schwarz-precond's public trait surface. CHANGELOG[Unreleased]updated.Test plan
cargo build --workspace --examples --testscargo test --workspace(all crates, including the nested-Rayon deadlock regressiontest_parallel_reduction_nested_rayon_does_not_deadlock)cargo clippy --workspace --all-targetscargo doc -p schwarz-precond --no-deps(crate has#![deny(missing_docs)])pixi run test(98 Python tests)