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

Use Julia v1.10 for CI #3403

Merged
merged 68 commits into from
Feb 27, 2024
Merged

Use Julia v1.10 for CI #3403

merged 68 commits into from
Feb 27, 2024

Conversation

navidcy
Copy link
Collaborator

@navidcy navidcy commented Dec 14, 2023

Closes #3427
Closes #3374

@navidcy navidcy added testing 🧪 Tests get priority in case of emergency evacuation package 📦 Quite meta labels Dec 14, 2023
@navidcy navidcy changed the title Use Julia v1.9.4 for CI Use Julia v1.10 for CI Dec 27, 2023
@navidcy
Copy link
Collaborator Author

navidcy commented Dec 30, 2023

Doctests fail because of all the loop not unrolled warnings we get.
We should remedy this before we move to Julia v1.10.

@glwagner you bumped onto this previously, right?

julia> using Oceananigans
Precompiling Oceananigans
  1 dependency successfully precompiled in 11 seconds. 143 already precompiled.
[ Info: Oceananigans will use 8 threads

julia> grid = RectilinearGrid(size=(1, 8, 8), extent=(1, 1, 1))
1×8×8 RectilinearGrid{Float64, Periodic, Periodic, Bounded} on CPU with 3×3×3 halo
├── Periodic x  [0.0, 1.0)  regularly spaced with Δx=1.0
├── Periodic y  [0.0, 1.0)  regularly spaced with Δy=0.125
└── Bounded  z  [-1.0, 0.0] regularly spaced with Δz=0.125

julia> model = NonhydrostaticModel(; grid)
warning: /Users/navid/.julia/packages/KernelAbstractions/WoCk1/src/extras/loopinfo.jl:28:0: loop not unrolled: the optimizer was unable to perform the requested transformation; the transformation might be disabled or specified as part of an unsupported transformation ordering
warning: /Users/navid/.julia/packages/KernelAbstractions/WoCk1/src/extras/loopinfo.jl:28:0: loop not unrolled: the optimizer was unable to perform the requested transformation; the transformation might be disabled or specified as part of an unsupported transformation ordering
warning: /Users/navid/.julia/packages/KernelAbstractions/WoCk1/src/extras/loopinfo.jl:28:0: loop not unrolled: the optimizer was unable to perform the requested transformation; the transformation might be disabled or specified as part of an unsupported transformation ordering
warning: /Users/navid/.julia/packages/KernelAbstractions/WoCk1/src/extras/loopinfo.jl:28:0: loop not unrolled: the optimizer was unable to perform the requested transformation; the transformation might be disabled or specified as part of an unsupported transformation ordering
warning: /Users/navid/.julia/packages/KernelAbstractions/WoCk1/src/extras/loopinfo.jl:28:0: loop not unrolled: the optimizer was unable to perform the requested transformation; the transformation might be disabled or specified as part of an unsupported transformation ordering
warning: /Users/navid/.julia/packages/KernelAbstractions/WoCk1/src/extras/loopinfo.jl:28:0: loop not unrolled: the optimizer was unable to perform the requested transformation; the transformation might be disabled or specified as part of an unsupported transformation ordering
warning: /Users/navid/.julia/packages/KernelAbstractions/WoCk1/src/extras/loopinfo.jl:28:0: loop not unrolled: the optimizer was unable to perform the requested transformation; the transformation might be disabled or specified as part of an unsupported transformation ordering
warning: /Users/navid/.julia/packages/KernelAbstractions/WoCk1/src/extras/loopinfo.jl:28:0: loop not unrolled: the optimizer was unable to perform the requested transformation; the transformation might be disabled or specified as part of an unsupported transformation ordering
warning: /Users/navid/.julia/packages/KernelAbstractions/WoCk1/src/extras/loopinfo.jl:28:0: loop not unrolled: the optimizer was unable to perform the requested transformation; the transformation might be disabled or specified as part of an unsupported transformation ordering
warning: /Users/navid/.julia/packages/KernelAbstractions/WoCk1/src/extras/loopinfo.jl:28:0: loop not unrolled: the optimizer was unable to perform the requested transformation; the transformation might be disabled or specified as part of an unsupported transformation ordering
warning: /Users/navid/.julia/packages/KernelAbstractions/WoCk1/src/extras/loopinfo.jl:28:0: loop not unrolled: the optimizer was unable to perform the requested transformation; the transformation might be disabled or specified as part of an unsupported transformation ordering
warning: /Users/navid/.julia/packages/KernelAbstractions/WoCk1/src/extras/loopinfo.jl:28:0: loop not unrolled: the optimizer was unable to perform the requested transformation; the transformation might be disabled or specified as part of an unsupported transformation ordering
warning: /Users/navid/.julia/packages/KernelAbstractions/WoCk1/src/extras/loopinfo.jl:28:0: loop not unrolled: the optimizer was unable to perform the requested transformation; the transformation might be disabled or specified as part of an unsupported transformation ordering
NonhydrostaticModel{CPU, RectilinearGrid}(time = 0 seconds, iteration = 0)
├── grid: 1×8×8 RectilinearGrid{Float64, Periodic, Periodic, Bounded} on CPU with 3×3×3 halo
├── timestepper: QuasiAdamsBashforth2TimeStepper
├── tracers: ()
├── closure: Nothing
├── buoyancy: Nothing
└── coriolis: Nothing

@navidcy
Copy link
Collaborator Author

navidcy commented Dec 30, 2023

this warnings are related to #3374, right?

@navidcy
Copy link
Collaborator Author

navidcy commented Jan 10, 2024

OK, now tests pass but I believe I removed more @unrolls than I should. It was only for testing purposes to see if those were the culprit for the warnings.

@glwagner could we zoom and put back in the @unrolls that we should?

@navidcy
Copy link
Collaborator Author

navidcy commented Jan 10, 2024

(This is an important PR in order to start using Julia v1.10 without concerns.)

@glwagner
Copy link
Member

glwagner commented Feb 6, 2024

Hmm, it should not be a tolerance issue. To debug this I would probably suggest Tartarus since it has at least 2 GPUs (I am not sure your laptop has 2 gpus 😄 )

By the way, for unrolling loops we can look at this package which might be very useful to us https://github.com/cstjean/Unrolled.jl

What's the difference between that and what we are using from KernelAbstractions.jl?

@glwagner
Copy link
Member

glwagner commented Feb 6, 2024

Hmm, it should not be a tolerance issue. To debug this I would probably suggest Tartarus since it has at least 2 GPUs (I am not sure your laptop has 2 gpus 😄 )

I checked and tests were failing because values of order 1e-19 or less were not agreeing... see https://buildkite.com/clima/oceananigans-distributed/builds/1131#018d7a2d-f6c5-4e22-8006-3e2d318465d1/170-5060

I replaced a ≈ b (which defaults to atol=0 with isapprox(a, b, atol=eps(eltype(grid))); see 385a05d

Makes sense. Are the differences associated with some of the unrolling that we added (eg to fill halo regions)?

@navidcy
Copy link
Collaborator Author

navidcy commented Feb 7, 2024

Makes sense. Are the differences associated with some of the unrolling that we added (eg to fill halo regions)?

We didn't add any unrolling, we only removed unrollings...

@navidcy
Copy link
Collaborator Author

navidcy commented Feb 7, 2024

oh I see, do you mean the proper use of Val(grid.Hx) e.g. in

launch!(arch, grid, KernelParameters(yz_size, offset), fill_periodic_west_and_east_halo!, c_parent, Val(grid.Hx), grid.Nx; kw...)

Unfortunately we don't know because the distributed CI was broken at that point; I only fixed it after with e8da741. To test this hypothesis we should remove the Val(grid.Hx) etc and see.

@navidcy
Copy link
Collaborator Author

navidcy commented Feb 14, 2024

This PR ready. I just wanted to wait until after OSM2024 to merge since it includes various deps updates as well.

Or, @glwagner, feel free to merge at will whenever.

@simone-silvestri
Copy link
Collaborator

I would wait a bit. I am seeing some instabilities with Julia 1.10.

@navidcy
Copy link
Collaborator Author

navidcy commented Feb 14, 2024

What sort of instabilities?

@simone-silvestri
Copy link
Collaborator

simone-silvestri commented Feb 14, 2024

I had problems with JSON3 and FFMPEG not compiling on mac and linux. I will probably write an issue.

@navidcy
Copy link
Collaborator Author

navidcy commented Feb 14, 2024

So non-Oceananigans related?

@simone-silvestri
Copy link
Collaborator

yep

@glwagner
Copy link
Member

But this PR doesn't require users to use 1.10 right? It's just about running the tests on 1.10.

I think it will be nice to get rid of the crazy warnings on 1.10.

@navidcy
Copy link
Collaborator Author

navidcy commented Feb 14, 2024

But this PR doesn't require users to use 1.10 right? It's just about running the tests on 1.10.

yes!

I think it will be nice to get rid of the crazy warnings on 1.10.

yes

@navidcy
Copy link
Collaborator Author

navidcy commented Feb 15, 2024

But this PR doesn't require users to use 1.10 right? It's just about running the tests on 1.10.

(Yes, but also the Manifest includes a lot of updates for various dependencies so that all pkgs versions are resolved with v1.10.)

@navidcy
Copy link
Collaborator Author

navidcy commented Feb 26, 2024

Shall we merge this (when tests pass)?

@navidcy navidcy merged commit 17ab145 into main Feb 27, 2024
48 checks passed
@navidcy navidcy deleted the ncc/use-julia-v1.9.4 branch February 27, 2024 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup 🧹 Paying off technical debt package 📦 Quite meta testing 🧪 Tests get priority in case of emergency evacuation
Projects
None yet
3 participants