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

CompatHelper: bump compat for GPUArrays to 11 and JLArrays to 0.2, (keep existing compat) #590

Conversation

github-actions[bot]
Copy link
Contributor

This pull request changes the compat entry for the GPUArrays package from 10 to 10, 11.
This keeps the compat entries for earlier versions.

Note: I have not tested your package with this new compat entry.
It is your responsibility to make sure that your package tests pass before you merge this pull request.

@navidcy navidcy force-pushed the compathelper/new_version/2024-10-18-01-40-44-523-02004937921 branch from ae1bdf5 to b8feeba Compare October 18, 2024 01:40
@milankl
Copy link
Member

milankl commented Oct 18, 2024

looks like we have to wait for a new CUDA.jl release? But I thought we have CUDA now as an extension with #586 ?

@maximilian-gelbrecht
Copy link
Member

maximilian-gelbrecht commented Oct 21, 2024

The tests instantiate CUDA, so that's why the CI fails. I don't know why we load it though. We could just remove CUDA from the test dependencies.

@maximilian-gelbrecht
Copy link
Member

Ok, this also fails, now because of JLArrays. Yeah, I guess we have to wait 🤷

@maximilian-gelbrecht
Copy link
Member

Ah wait, we had JLArrays restricted to 0.1.4, which isn't compatible with GPUArras v11. I removed that now, but I don't understand why it still can't resolve the dependencies.

@maximilian-gelbrecht
Copy link
Member

On my laptop it's fine and doesn't complain about these dependencies.

@maximilian-gelbrecht maximilian-gelbrecht added the dependency 🦮 Dependencies on other packages label Oct 22, 2024
@maximilian-gelbrecht
Copy link
Member

The issue is that JLArrays wasn't registered yet in the newest version. As soon as it is, the dependency problem here should be resolved.

At the same time there were some changes to GPUArrays. So this rescricts combat to v11.

@milankl
Copy link
Member

milankl commented Nov 28, 2024

There's a package conflict between GPUArrays and JLArrays

 caused by: Unsatisfiable requirements detected for package JLArrays [27aeb0d3]:
 JLArrays [27aeb0d3] log:
 ├─possible versions are: 0.1.0-0.2.0 or uninstalled
 ├─restricted to versions 0.1 by SpeedyWeather [9e226e20], leaving only versions: 0.1.0-0.1.6 or uninstalled
 │ └─SpeedyWeather [9e226e20] log:
 │   ├─possible versions are: 0.12.1 or uninstalled
 │   └─SpeedyWeather [9e226e20] is fixed to version 0.12.1
 ├─restricted to versions 0.1 by project [ab5e48d3], leaving only versions: 0.1.0-0.1.6
 │ └─project [ab5e48d3] log:
 │   ├─possible versions are: 0.0.0 or uninstalled
 │   └─project [ab5e48d3] is fixed to version 0.0.0
 └─restricted by compatibility requirements with GPUArrays [0c68f7d7] to versions: 0.2.0 or uninstalled — no versions left
   └─GPUArrays [0c68f7d7] log:
     ├─possible versions are: 0.3.0-11.1.0 or uninstalled
     ├─restricted to versions 11 by SpeedyWeather [9e226e20], leaving only versions: 11.0.0-11.1.0
     │ └─SpeedyWeather [9e226e20] log: see above
     └─restricted to versions 11.1.0 by an explicit requirement, leaving only versions: 11.1.0

given that this is essentially the same conflict as in #605 I reckon compat for both GPUArrays and JLArrays has to be bumped up simultaneously?

@milankl milankl added the compatibility 👯 Changes necessary for compatibility between versions label Nov 28, 2024
@milankl milankl changed the title CompatHelper: bump compat for GPUArrays to 11, (keep existing compat) CompatHelper: bump compat for GPUArrays to 11 and JLArrays to 0.2, (keep existing compat) Nov 28, 2024
@milankl
Copy link
Member

milankl commented Nov 28, 2024

 ┌ SpeedyWeather  SpeedyWeatherJLArraysExt
│  WARNING: Method definition (::Type{SpeedyWeather.ColumnVariables{NF, VectorType, MatrixType} where MatrixType where VectorType where NF})() in module SpeedyWeather at /home/runner/work/SpeedyWeather.jl/SpeedyWeather.jl/src/physics/define_column.jl:10 overwritten at /home/runner/work/SpeedyWeather.jl/SpeedyWeather.jl/src/physics/define_column.jl:106.
│  ERROR: Method overwriting is not permitted during Module precompilation. Use `__precompile__(false)` to opt-out of precompilation.
└  
┌ SpeedyWeather
│  WARNING: Method definition (::Type{SpeedyWeather.ColumnVariables{NF, VectorType, MatrixType} where MatrixType where VectorType where NF})() in module SpeedyWeather at /home/runner/work/SpeedyWeather.jl/SpeedyWeather.jl/src/physics/define_column.jl:10 overwritten at /home/runner/work/SpeedyWeather.jl/SpeedyWeather.jl/src/physics/define_column.jl:106.
│  ERROR: Method overwriting is not permitted during Module precompilation. Use `__precompile__(false)` to opt-out of precompilation.
└ 

ooops too many generator methods!

@maximilian-gelbrecht
Copy link
Member

Yeah this is just a bit annoying. JLArrays is a sub-package of GPUArrays, so the dependency error is just a bit odd. I hoped this kind of fixes itself with new releases of all packages. Will take another look at this next week.

@milankl
Copy link
Member

milankl commented Nov 29, 2024

 ┌ SpeedyWeather  SpeedyWeatherJLArraysExt
│  WARNING: Method definition (::Type{SpeedyWeather.ColumnVariables{NF, VectorType, MatrixType} where MatrixType where VectorType where NF})() in module SpeedyWeather at /home/runner/work/SpeedyWeather.jl/SpeedyWeather.jl/src/physics/define_column.jl:10 overwritten at /home/runner/work/SpeedyWeather.jl/SpeedyWeather.jl/src/physics/define_column.jl:106.
│  ERROR: Method overwriting is not permitted during Module precompilation. Use `__precompile__(false)` to opt-out of precompilation.
└  
┌ SpeedyWeather
│  WARNING: Method definition (::Type{SpeedyWeather.ColumnVariables{NF, VectorType, MatrixType} where MatrixType where VectorType where NF})() in module SpeedyWeather at /home/runner/work/SpeedyWeather.jl/SpeedyWeather.jl/src/physics/define_column.jl:10 overwritten at /home/runner/work/SpeedyWeather.jl/SpeedyWeather.jl/src/physics/define_column.jl:106.
│  ERROR: Method overwriting is not permitted during Module precompilation. Use `__precompile__(false)` to opt-out of precompilation.
└ 

ooops too many generator methods!

Removed in #618

@milankl
Copy link
Member

milankl commented Nov 29, 2024

Zeros, ones, rand, and randn constructors: Error During Test at /home/runner/work/SpeedyWeather.jl/SpeedyWeather.jl/test/lower_triangular_matrix.jl:299
  Test threw exception
  Expression: all(JL2 .== JL)
  StackOverflowError:
  Stacktrace:
   [1] get_backend(A::LowerTriangularArray{Bool, 2, JLArray{Bool, 2}}) (repeats 79984 times)
     @ KernelAbstractions ~/.julia/packages/KernelAbstractions/iW1Rw/src/KernelAbstractions.jl:466

this looks like a dispatch loop to me, with some conflict between the new GPUArrays and KernelAbstractions? @vchuravy you know?

@maximilian-gelbrecht
Copy link
Member

Dependency issues seem to be resolved now (at least running this on my laptop), I'll hopefully merge this tomorrow and take a look at all the other missing compat entries that the bot and warning pointed out

@milankl
Copy link
Member

milankl commented Dec 10, 2024

@maximilian-gelbrecht after this being merged, @jackleland is facing a circular dependency on main

julia> import CUDA

julia> import SpeedyWeather
┌ Warning: Circular dependency detected. Precompilation will be skipped for:
│   SparseArraysExt [85068d23-b5fb-53f1-8204-05c2aba6942f]
│   SpecialFunctionsExt [1285c0f1-ff9f-5867-b66e-0f359bcf09ba]
│   AtomixCUDAExt [13011619-4c7c-5ef0-948f-5fc81565cd05]
│   LinearAlgebraExt [66d79d19-2cc4-5b0b-ac7a-b340256d1ecd]
│   KernelAbstractions [63c18a36-062a-441e-b654-da1e3ab1ce7c]
│   SpeedyWeatherCUDAExt [6a391560-0780-5311-8181-58d371f98820]
│   CUDA [052768ef-5323-5732-b1bb-66c8b64840ba]
│   SpeedyWeather [9e226e20-d153-4fed-8a5b-493def4f21a9]
└ @ Pkg.API ~/.julia/juliaup/julia-1.10.4+0.x64.linux.gnu/share/julia/stdlib/v1.10/Pkg/src/API.jl:1239
[ Info: Precompiling SpeedyWeatherCUDAExt [6a391560-0780-5311-8181-58d371f98820]

julia> methods(SpeedyWeather.SpeedyTransforms._fourier!)
# 2 methods for generic function "_fourier!" from SpeedyWeather.SpeedyTransforms:
 [1] _fourier!(grids::SpeedyWeather.RingGrids.AbstractGridArray, f_north, f_south, S::SpeedyWeather.SpeedyTransforms.SpectralTransform)
     @ ~/notebooks/SpeedyWeather.jl/src/SpeedyTransforms/fourier.jl:12
 [2] _fourier!(f_north, f_south, grids::SpeedyWeather.RingGrids.AbstractGridArray, S::SpeedyWeather.SpeedyTransforms.SpectralTransform)
     @ ~/notebooks/SpeedyWeather.jl/src/SpeedyTransforms/fourier.jl:2

you have that too?

@maximilian-gelbrecht
Copy link
Member

Uargh ..., I can't even add both packages in a temp env due to a dependency problem.

These dependency problems with the GPUArrays update are really annoying. I thought it's resolved now.

This deserves urgent attention. Today and tomorrow are really full days for me, but I'll look into it Thursday/Friday.

@milankl
Copy link
Member

milankl commented Dec 10, 2024

Mhhh?

(@v1.11) pkg> add CUDA
   Resolving package versions...
ERROR: Unsatisfiable requirements detected for package CUDA [052768ef]:
 CUDA [052768ef] log:
 ├─possible versions are: 0.1.0 - 5.5.2 or uninstalled
 ├─restricted to versions 4 - 5 by SpeedyWeather [9e226e20], leaving only versions: 4.0.0 - 5.5.2 or uninstalled
 │ └─SpeedyWeather [9e226e20] log:
 │   ├─possible versions are: 0.13.0 or uninstalled
 │   └─SpeedyWeather [9e226e20] is fixed to version 0.13.0
 ├─restricted to versions * by an explicit requirement, leaving only versions: 4.0.0 - 5.5.2
 └─restricted by compatibility requirements with GPUArrays [0c68f7d7] to versions: uninstalled — no versions left
   └─GPUArrays [0c68f7d7] log:
     ├─possible versions are: 0.3.0 - 11.1.0 or uninstalled
     └─restricted to versions 11 by SpeedyWeather [9e226e20], leaving only versions: 11.0.0 - 11.1.0
       └─SpeedyWeather [9e226e20] log: see above

@jackleland
Copy link
Collaborator

Mhhh?

(@v1.11) pkg> add CUDA
   Resolving package versions...
ERROR: Unsatisfiable requirements detected for package CUDA [052768ef]:
 CUDA [052768ef] log:
 ├─possible versions are: 0.1.0 - 5.5.2 or uninstalled
 ├─restricted to versions 4 - 5 by SpeedyWeather [9e226e20], leaving only versions: 4.0.0 - 5.5.2 or uninstalled
 │ └─SpeedyWeather [9e226e20] log:
 │   ├─possible versions are: 0.13.0 or uninstalled
 │   └─SpeedyWeather [9e226e20] is fixed to version 0.13.0
 ├─restricted to versions * by an explicit requirement, leaving only versions: 4.0.0 - 5.5.2
 └─restricted by compatibility requirements with GPUArrays [0c68f7d7] to versions: uninstalled — no versions left
   └─GPUArrays [0c68f7d7] log:
     ├─possible versions are: 0.3.0 - 11.1.0 or uninstalled
     └─restricted to versions 11 by SpeedyWeather [9e226e20], leaving only versions: 11.0.0 - 11.1.0
       └─SpeedyWeather [9e226e20] log: see above

I fixed this for myself by relaxing GPUArrays compat to [10, 11] (not quite sure why it was capped at 11?), but the circular dependency problem remains

@maximilian-gelbrecht
Copy link
Member

Mhhh?

(@v1.11) pkg> add CUDA
   Resolving package versions...
ERROR: Unsatisfiable requirements detected for package CUDA [052768ef]:
 CUDA [052768ef] log:
 ├─possible versions are: 0.1.0 - 5.5.2 or uninstalled
 ├─restricted to versions 4 - 5 by SpeedyWeather [9e226e20], leaving only versions: 4.0.0 - 5.5.2 or uninstalled
 │ └─SpeedyWeather [9e226e20] log:
 │   ├─possible versions are: 0.13.0 or uninstalled
 │   └─SpeedyWeather [9e226e20] is fixed to version 0.13.0
 ├─restricted to versions * by an explicit requirement, leaving only versions: 4.0.0 - 5.5.2
 └─restricted by compatibility requirements with GPUArrays [0c68f7d7] to versions: uninstalled — no versions left
   └─GPUArrays [0c68f7d7] log:
     ├─possible versions are: 0.3.0 - 11.1.0 or uninstalled
     └─restricted to versions 11 by SpeedyWeather [9e226e20], leaving only versions: 11.0.0 - 11.1.0
       └─SpeedyWeather [9e226e20] log: see above

I fixed this for myself by relaxing GPUArrays compat to [10, 11] (not quite sure why it was capped at 11?), but the circular dependency problem remains

It's capped at 11 because GPUArrays syntax changed from 10 to 11. We use the package to make our RingGrids and LowerTriangularArray broadcast work on GPU, and the syntax for that changed.

@milankl
Copy link
Member

milankl commented Dec 10, 2024

relaxing GPUArrays compat to [10, 11]

where? In SpeedyWeather? but that causes problem with the backend vs get_backend functions no? But maybe I don't see but latest CUDA is happy with GPUArrays v11, SpeedyWeather is happy with GPUArrays v11 and SpeedyWeather is happy with the latest CUDA, so what's the problem?

No only CUDA main is happy with GPUArrays v11, CUDA v5.5.2 (latest release) isn't...

@maximilian-gelbrecht
Copy link
Member

maximilian-gelbrecht commented Dec 10, 2024

If that's the issue we can just ask for a new minor CUDA.jl release probably.

@milankl
Copy link
Member

milankl commented Dec 10, 2024

JuliaGPU/CUDA.jl#2587

@jackleland
Copy link
Collaborator

@maximilian-gelbrecht @milankl Fixed my circular dependency problem by rolling KernelAbstractions back to 0.9.29

@milankl
Copy link
Member

milankl commented Dec 11, 2024

Awesome. Should we get a pull request with compat GPUArrays <v11 and KernelAbstractions 0.9.29 in? We could even tag it as v0.13.1

@maximilian-gelbrecht
Copy link
Member

Yes. Sorry, very busy week. I can take care of it. (I'll also have a look at the GPU/AD progress some time next week).

@maximilian-gelbrecht
Copy link
Member

#636

@maximilian-gelbrecht
Copy link
Member

@maximilian-gelbrecht @milankl Fixed my circular dependency problem by rolling KernelAbstractions back to 0.9.29

Any idea what caused this? I included it in the #636 PR though

In other projects I also have massive issues with circular dependency at the moment (primarily from many extensions in the SciML space etc)

@jackleland
Copy link
Collaborator

jackleland commented Dec 16, 2024

Not entirely sure, but I think it's a bug with julia introduced by trying to simplify extension loading (JuliaLang/julia#55589). I think it's basically a problem with your dependencies depending on each other but, again, it's not entirely clear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compatibility 👯 Changes necessary for compatibility between versions dependency 🦮 Dependencies on other packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants