refactor(solver): dedup factorization caching and cross-DB dep helpers#98
Merged
Merged
Conversation
SharedSolver carried several copies of the same logic. Factor them out: - Lazy MUMPS factorization was computed-and-cached in two near-identical blocks (solveWithSharedSolver, ensureFactorization). Extract computeAndStoreFactorization so the precompute+store lives once; the dense-solver fallback stays scoped to the first-solve (cache-miss) path. - The per-root dep-demand -> demand-vector conversion was copied byte for byte in three resolvers (resolveDep, resolveDepContribs, and Service.resolveDepWithSubs). Extract prepareDepDemandVecs, plus depDbsOf for the "DBs referenced at this level" set. - CrossDBSolution gains a Semigroup instance, so mergeSolutions is just foldl' (<>) — same summed inventory and BFS-ordered scalings as before. - Name the cross-DB dep-demand map types in Matrix (SupplierDemands, DepDemands) and use them across the affected signatures. No behaviour change; full test suite (1107 examples) green.
The factorization/merge dedup left modifyMVar (superseded by modifyMVar_) and foldl' (provided by Prelude) unused, producing -Wall warnings.
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
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
A readability pass over
SharedSolver.hsthat removes duplicated logic without changing behaviour.solveWithSharedSolver,ensureFactorization). ExtractedcomputeAndStoreFactorizationso the precompute + store lives once; the dense-solver fallback stays scoped to the first-solve (cache-miss) path exactly as before.resolveDep,resolveDepContribs, andService.resolveDepWithSubs). ExtractedprepareDepDemandVecs, plusdepDbsOffor the "DBs referenced at this level" set.Semigroup CrossDBSolution.mergeSolutionsis nowfoldl' (<>)— same summed inventory and base-first / BFS-ordered scalings as the hand-written version.SupplierDemands/DepDemandsaliases inMatrix(where the type is produced) and used them across the affected signatures, replacing the verbose nested-Mapspelling.We deliberately stopped short of unifying the two dependency-graph walks (inventory vs. contributions) behind a generic interpreter: it barely reduces line count, would rely on a K=1 invariant the types don't enforce, and can't absorb the third walk in
Service.hs.Test plan
./gen-version.sh && cabal build— clean (only two pre-existing, unrelatedAPI/MCP.hswarnings).cabal run lca-tests— 1107 examples, 0 failures, 1 pending.advanced_haskell; rebuilt and re-ran the full suite green on the rebased tree.