Skip to content

Conversation

JoshuaLampert
Copy link
Member

@JoshuaLampert JoshuaLampert commented Mar 24, 2025

I just switched to KLUFactorization() instead of LUFactorization(). Let's see if CI passes. Do we generally prefer LUFactorization() over KLUFactorization()? So should we continue to search for a fix for LUFactorization() or should we change the default to KLUFactorization() at least for now to let the tests pass again (would be breaking though)?

Using KLUFactorization() instead of LUFactorization for sparse matrices fixes the pattern of the matrix changed error from LinearSolve.jl v3.
Closes #131, closes #135, closes #136, closes #137, closes #150, closes #151, closes #152.

@JoshuaLampert JoshuaLampert marked this pull request as draft March 24, 2025 10:24
@JoshuaLampert JoshuaLampert marked this pull request as ready for review March 24, 2025 12:41
@codecov-commenter
Copy link

codecov-commenter commented Mar 24, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.92%. Comparing base (d8a3c61) to head (fa4b682).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #153   +/-   ##
=======================================
  Coverage   97.92%   97.92%           
=======================================
  Files           7        7           
  Lines        1641     1641           
=======================================
  Hits         1607     1607           
  Misses         34       34           

☔ View full report in Codecov by Sentry.
📢 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.

@JoshuaLampert
Copy link
Member Author

This is ready for a review. Regular CI is passing. For the Downgrade test I'll see if I can debug this in another PR.

@JoshuaLampert
Copy link
Member Author

To clarify, this PR is also only kind of a hotfix as in general we should be able to solve sparse problems with LUFactorization. If we want to merge this, we also need to think about refactoring https://skopecz.github.io/PositiveIntegrators.jl/stable/heat_equation_dirichlet/#Performance-comparison as we compare both (LUFactorization abd KLUFactorization) there.

Copy link
Member

@ranocha ranocha left a comment

Choose a reason for hiding this comment

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

Thanks for all the work!

@ranocha
Copy link
Member

ranocha commented Mar 25, 2025

To clarify, this PR is also only kind of a hotfix as in general we should be able to solve sparse problems with LUFactorization. If we want to merge this, we also need to think about refactoring https://skopecz.github.io/PositiveIntegrators.jl/stable/heat_equation_dirichlet/#Performance-comparison as we compare both (LUFactorization abd KLUFactorization) there.

We need to discuss this - the docs fail because of this (https://github.com/SKopecz/PositiveIntegrators.jl/actions/runs/14036192931/job/39294690579?pr=153#step:7:27)

@JoshuaLampert JoshuaLampert mentioned this pull request Mar 27, 2025
9 tasks
@JoshuaLampert
Copy link
Member Author

There are more and more errors, when we try to keep support of LinearSolve.jl v2, see https://github.com/SKopecz/PositiveIntegrators.jl/actions/runs/14127496106/job/39579796614?pr=153#step:8:425 for the option, where we require LinearSolve v2.39.1 as suggested by @ChrisRackauckas. I don't understand where the error comes from, but also don't want to spend the time to debug this. So let's just drop support for LinearSolve.jl v2.

@JoshuaLampert JoshuaLampert requested a review from ranocha March 28, 2025 13:08
Copy link
Member

@ranocha ranocha left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@ranocha ranocha merged commit 569050e into NumericalMathematics:main Mar 28, 2025
10 checks passed
@JoshuaLampert JoshuaLampert deleted the linearsolve-3 branch March 28, 2025 15:18
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.

Remove hotfix for failing tests related to LinearSolve v2
4 participants