Skip to content

Standardize solver refactoring and shared factorizations - #55

Merged
jcapriot merged 3 commits into
maint/deprecationsfrom
reuse_factorization
Sep 30, 2026
Merged

jcapriot merged 3 commits into
maint/deprecationsfrom
reuse_factorization

Conversation

@jcapriot

Copy link
Copy Markdown
Member

Introduces a public factor(A=None) method across all solvers for consistent matrix updates. Some solvers already supported this. This just makes it a guaranteed interface across all solvers. This enables efficient re-factorization when the matrix values change but the sparsity pattern and analysis can be reused.

For direct solvers like Mumps and Pardiso, a new _SharedFactorBase ensures that transposed and conjugated views share the same underlying factorization. This was to make it so that when the underlying solver object is refactored, any other transposed or conjugate views stay in sync with the updated solver.

@jcapriot
jcapriot added this pull request to stack #56 September 27, 2026 21:23
@codecov

codecov Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.45946% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 99.66%. Comparing base (200d44b) to head (ea89bb2).

Files with missing lines Patch % Lines
tests/test_Wrappers.py 94.11% 1 Missing ⚠️
Additional details and impacted files
@@                  Coverage Diff                   @@
##           maint/deprecations      #55      +/-   ##
======================================================
- Coverage              100.00%   99.66%   -0.34%     
======================================================
  Files                      16       17       +1     
  Lines                    1059     1197     +138     
======================================================
+ Hits                     1059     1193     +134     
- Misses                      0        4       +4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Introduces a public `factor(A=None)` method across all solvers for consistent matrix updates. For direct solvers like `Mumps` and `Pardiso`, a new `_SharedFactorBase` ensures that transposed and conjugated views share the same underlying factorization. This enables efficient re-factorization when the matrix values change but the sparsity pattern and analysis can be reused.

@santisoler santisoler left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good. My only comment would be why instead of "stacking" the inheritance like:

Base --(parent of)--> _SharedFactorBase --(parent of)--> Mumps

we make multiple inheritance like:

class Mumps(_SharedFactorBase, Base):
    ...

We could even make _SharedFactorBase an ABC class and force child classes to implement the factor method.

Just an idea, a bit of nit picking too. What do you think?

@jcapriot

Copy link
Copy Markdown
Member Author

It looks good. My only comment would be why instead of "stacking" the inheritance like:

Base --(parent of)--> _SharedFactorBase --(parent of)--> Mumps

we make multiple inheritance like:

class Mumps(_SharedFactorBase, Base):
    ...

We could even make _SharedFactorBase an ABC class and force child classes to implement the factor method.

Just an idea, a bit of nit picking too. What do you think?

I like the idea of making it a mix in class. I don't think the factor method is what distinguishes it though. These are meant for classes that wrap some other solver that owns the factorization, not necessarily if they can be factored.

@jcapriot
jcapriot merged commit 717dff3 into main Sep 30, 2026
58 of 60 checks passed
@jcapriot
jcapriot deleted the reuse_factorization branch September 30, 2026 17:26
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