Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Circular dependency on 1.10 #573

Closed
j-fu opened this issue Feb 7, 2025 · 16 comments · Fixed by #577 · May be fixed by JuliaArrays/LazyArrays.jl#365
Closed

Circular dependency on 1.10 #573

j-fu opened this issue Feb 7, 2025 · 16 comments · Fixed by #577 · May be fixed by JuliaArrays/LazyArrays.jl#365
Labels
bug Something isn't working

Comments

@j-fu
Copy link
Contributor

j-fu commented Feb 7, 2025

Describe the bug 🐞

I am getting circular dependency errors with LinearSolve v3 on Julua 1.10 (but not on 1.11 and nightly).
In particular, this makes the Aqua test for persistent tasks fail.

Expected behavior

Aqua test for persistent tasks works.

Minimal Reproducible Example 👇

See https://github.com/j-fu/LSolveMWE

Pkg.test the package under 1.10 vs 1.11.

Error & Stacktrace ⚠️
This is the test output on 1.10:

┌ Warning: Circular dependency detected.
│ Precompilation will be skipped for dependencies in this cycle:
│  ┌ LinearSolve → LinearSolveSparseArraysExt
│  └─ LinearSolve → LinearSolveRecursiveArrayToolsExt
│ Precompilation will also be skipped for the following, which depend on the above cycle:
│   jl_CjwJIzUtOD
│   LSolveMWE
└ @ Pkg.API.Precompilation ~/.julia/juliaup/julia-1.10.8+0.x64.linux.gnu/share/julia/stdlib/v1.10/Pkg/src/precompilation.jl:584
┌ Error: Unexpected error: /tmp/jl_CjwJIzUtOD/done.log was not created, but precompilation exited
└ @ Aqua ~/.julia/packages/Aqua/epbUr/src/persistent_tasks.jl:118
Test Failed at /home/fuhrmann/.julia/packages/Aqua/epbUr/src/persistent_tasks.jl:38
Expression: !(has_persistent_tasks(package; kwargs...))

ERROR: LoadError: There was an error during testing
in expression starting at /home/fuhrmann/Wias/work/julia/test/LSolveMWE/test/runtests.jl:5
ERROR: Package LSolveMWE errored during testing

Environment (please complete the following information):

See the CI results in https://github.com/j-fu/LSolveMWE

Additional context

@jpthiele

@j-fu j-fu added the bug Something isn't working label Feb 7, 2025
@j-fu
Copy link
Contributor Author

j-fu commented Feb 7, 2025

Ok, this seems to be related to something like this:
JuliaLang/julia#52511 (comment)

I updated the MWE to remove the SparseArrays dependency of the project, and
it is still in the manifest:

(LSolveMWE) pkg> why SparseArrays
LinearSolve → Krylov → SparseArrays
LinearSolve → LazyArrays → SparseArrays
LinearSolve → SciMLBase → RecursiveArrayTools → Statistics → SparseArrays
LinearSolve → SciMLBase → Statistics → SparseArrays
LinearSolve → SparseArrays

JuliaSmoothOptimizers/Krylov.jl#955 seems to take care of the first case...

@albertomercurio
Copy link

Same issue here.

@ufechner7
Copy link

Same issue here, in KiteModels.jl.

@oameye
Copy link

oameye commented Feb 11, 2025

Also happens for us.

@oscardssmith
Copy link
Contributor

This problem is very annoying. What happens is that on 1.11, SparseArrays isn't loaded by default, so it's possible to have a weakdep on it, but on 1.10, SparseArrays is in the sysimage so there's circularity. I think this is a Julia level issue.

@j-fu
Copy link
Contributor Author

j-fu commented Feb 11, 2025

It seems that the problem goes away if all packages LinearSolve depends on avoid having SparseArrays as strong dependency. I tested this locally.

All packages involved (Krylov, Statistics, LazyArrays) have now PRs/Issues addressing this point.

@oscardssmith
Copy link
Contributor

@ChrisRackauckas the simplest solution here would be to make RecursiveArrayTools a regular dep rather than a weakdep (which it effectively already is since SciMLBase has a strong dep on it)

@j-fu
Copy link
Contributor Author

j-fu commented Feb 11, 2025

However, the problem pops up again if we add e.g. ILUZero or AlgebraicMultigrid. These have
to depend on SparseArrays.
So the question then is: is there anything which can be backported from 1.11 to 1.10.9 which
resolves this problem ?

EDIT: this seems to be quite nontrivial: JuliaLang/julia#56204 (comment) - it appears that there was already a backport to 1.11 ...

@j-fu
Copy link
Contributor Author

j-fu commented Feb 12, 2025

Just another info bit: Aqua disables the corresponding test for Julia <1.10:
https://github.com/JuliaTesting/Aqua.jl/blob/3e90d046f718efaf04108913f8fb385f44026498/src/persistent_tasks.jl#L78

So a band-aid could be to disable this test as well for 1.10 and to live with the cyclic dependency warnings...

@ChrisRackauckas
Copy link
Member

We could keep a v1.11 branch, and we could make every release both have a minor and a patch, so like 3.1.0 and 3.1.1, where 3.1.0 is >= Julia v1.10 and has the hard SparseArrays dep, and then v3.1.1 has the weak dep. We will then need to do this in every SciML package and be disciplined about it. I don't really see another option.

@oscardssmith
Copy link
Contributor

should be fixed by #577

@topolarity
Copy link

Just for posterity in case anyone else stumbles on this issue:

1.10 has cycle problems still, esp. with "redundant extensions" (in this case LinearSolveRecursiveArrayToolsExt). It's "redundant" because it can be loaded by using LinearSolve alone - because LinearSolve depends on SciMLBase which depends on RecursiveArrayTools.

The fix in #577 is just to get rid of this extension and run its code always, which is what was happening anyway.

@aml5600
Copy link

aml5600 commented Feb 12, 2025

Unsure if related, but am hitting an error when doing Pkg operations on 1.10:

julia> Pkg.build()
ERROR: KeyError: key "SparseArrays" not found

I hit this with only ModelingToolkit in my Project. Stepping through Pkg.build() with the debugger I can see that when looping over the deps in the Manifest, it is on LinearSolve. It is then looping over extensions, and hitting LinearSolveSparseArraysExt, which then looks in the package's weakdeps, and SparseArrays is missing.

Does SparseArrays need to be added to weakdeps for the extension?

(if I should make a separate issue for this, please let me know. thanks!)

@topolarity
Copy link

Unsure if related, but am hitting an error when doing Pkg operations on 1.10:

That sounds like a Pkg bug that was in older versions of 1.10

Are you running on 1.10.8? If you are, please open another issue and we'll take a look

@aml5600
Copy link

aml5600 commented Feb 13, 2025

Unsure if related, but am hitting an error when doing Pkg operations on 1.10:

That sounds like a Pkg bug that was in older versions of 1.10

Are you running on 1.10.8? If you are, please open another issue and we'll take a look

Was actually on 1.10.7, updating helped. Thanks and sorry for the unrelated noise!

Still slightly curious why SparseArrays is the only extension not listed as a weakdep though?

@KristofferC
Copy link

but on 1.10, SparseArrays is in the sysimage

I don't think that is true actually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
9 participants