-
Notifications
You must be signed in to change notification settings - Fork 20
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 + CairoMakie v0.11 #166
Conversation
Actually we should better wait for CliMA/Oceananigans.jl#3403 |
Should we attempt to get rid of all of the @unrolls that are giving warnings |
I removed a few. The rest are in Oceananigans and CliMA/Oceananigans.jl#3403 should clear them up. |
@@ -27,7 +27,7 @@ | |||
end | |||
|
|||
# the rest of the points | |||
@unroll for k in grid.Nz-1:-1:1 | |||
for k in grid.Nz-1:-1:1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@navidcy for e.g. this @unroll
, do we have any idea how much time it would save to have it work? Because here (for example) we could give the function Nz in some way that it knows it at compile time, but the extra obfuscation in the code wouldn't be worth it if its not a great time saving
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no idea but you can try and bench?
I think we are ready for this PR since CliMA/Oceananigans.jl#3403 was merged. |
Sorry for the slow response to this. Does the current version of Oceananigans have some warnings left like in the tests or are they coming from OceanBioME? |
We need to retrigger the tests so they use v0.90.9 |
I restarted them now |
I don't understand why the CI has trouble installing Oceananigans v0.90.10 Also, what triggers this warning? https://buildkite.com/oceanbiome/oceanbiome/builds/134#018e2e08-1b82-42e2-8aa9-5a627152001d/25-28 |
OK, now some actual tests are failing. https://buildkite.com/oceanbiome/oceanbiome/builds/139#018e2e20-3791-4893-aa3a-a5772bc0c7ed/19-383 Any idea @jagoosw? |
I see you've resolved it now but I am also having trouble compiling 0.90.10 locally, when I update my julia to 1.10 it is resolved but might be something I should mention on the other repo. I'll try and work out whats going on with that test now. |
I think this is ready to go. I updated to Oceananigansv v0.90.11 that removed some of the unroll-related warnings that had sneaked in by accident. @jagoosw? |
Great, thank you for sorting this! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
bumping up a patch release so that we don't get warnings for deprecated Oceananigans method `arch_array` x-ref #166
use Julia v1.10 for CI and docs
also uses CairoMakie v0.11 and updates
Figure(resolution=...)
toFigure(size=...)
.