-
Notifications
You must be signed in to change notification settings - Fork 230
ContinuousForcing
not compiling on GPU with hydrostatic model on a lat-lon grid
#4165
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
Comments
And confirming that the rectilinear + non-hydrostatic case works fine and does not error: using Oceananigans
grid = RectilinearGrid(GPU();
topology = (Bounded, Bounded, Bounded),
size = (10, 10, 10),
x = (0, 10),
y = (-5, 5),
z = (-100, 0)
)
@inline relax(x, y, z, t, u, p) = - p.rate * (u - p.u★)
params = (
rate = 1.0,
u★ = 0.0
)
u_forcing = Forcing(relax; parameters=params, field_dependencies=:u)
forcing = (;
u = u_forcing
)
model = NonhydrostaticModel(; grid, forcing)
simulation = Simulation(model, Δt=0.01, stop_iteration=1)
run!(simulation) |
Also confirming that rectilinear + hydrostatic case works fine and does not error: using Oceananigans
grid = RectilinearGrid(GPU();
topology = (Bounded, Bounded, Bounded),
size = (10, 10, 10),
x = (0, 10),
y = (-5, 5),
z = (-100, 0)
)
@inline relax(x, y, z, t, u, p) = - p.rate * (u - p.u★)
params = (
rate = 1.0,
u★ = 0.0
)
u_forcing = Forcing(relax; parameters=params, field_dependencies=:u)
forcing = (;
u = u_forcing
)
model = HydrostaticFreeSurfaceModel(; grid, forcing)
simulation = Simulation(model, Δt=0.01, stop_iteration=1)
run!(simulation) |
Hmmm, I wonder if it has anything to do with Oceananigans.jl/src/Forcings/continuous_forcing.jl Lines 158 to 163 in 7afb057
|
The problem is fixed in #4008. Sorry that PR has been taking quite some time because the tests do not want to pass |
But I wonder actually now that I see that you are trying to use u as a field dependency (which should work regardless), if this bug is another bug which is exactly why I am not able to merge #4008 |
This seems like a type inference problem. As for One thing I'm not sure is why |
Does |
Might be worth getting rid of the splatting here:
we've had trouble with that before. |
Yes! I agree it feels like something weird is happening with the |
Thanks for the suggestions @simone-silvestri and @glwagner! I'll try them and see if I can get the MWE to compile. |
@simone-silvestri Just confirming that the MWE from this issue still produces the same error on PR #4008 so it might be a different cause :( |
Hmmm, this didn't help unfortunately but will try a few more tricks to help ease type inference. diff --git a/src/Forcings/continuous_forcing.jl b/src/Forcings/continuous_forcing.jl
index 388ed3958..c6c14188e 100644
--- a/src/Forcings/continuous_forcing.jl
+++ b/src/Forcings/continuous_forcing.jl
@@ -136,9 +136,9 @@ end
args = user_function_arguments(i, j, k, grid, model_fields, forcing.parameters, forcing)
- X = node(i, j, k, grid, LX(), LY(), LZ())
+ x, y, z = node(i, j, k, grid, LX(), LY(), LZ())
- return forcing.func(X..., clock.time, args...)
+ return forcing.func(x, y, z, clock.time, args...)
end
"""Show the innards of a `ContinuousForcing` in the REPL."""
@@ -155,12 +155,12 @@ Base.show(io::IO, forcing::ContinuousForcing{Nothing, Nothing, Nothing, P}) wher
"├── parameters: $(forcing.parameters)", "\n",
"└── field dependencies: $(forcing.field_dependencies)")
-Adapt.adapt_structure(to, forcing::ContinuousForcing{LX, LY, LZ}) where {LX, LY, LZ} =
- ContinuousForcing{LX, LY, LZ}(Adapt.adapt(to, forcing.func),
- Adapt.adapt(to, forcing.parameters),
- nothing,
- Adapt.adapt(to, forcing.field_dependencies_indices),
- Adapt.adapt(to, forcing.field_dependencies_interp))
+Adapt.adapt_structure(to, forcing::ContinuousForcing)=
+ ContinuousForcing{Nothing, Nothing, Nothing}(Adapt.adapt(to, forcing.func),
+ Adapt.adapt(to, forcing.parameters),
+ nothing,
+ Adapt.adapt(to, forcing.field_dependencies_indices),
+ Adapt.adapt(to, forcing.field_dependencies_interp))
on_architecture(to, forcing::ContinuousForcing{LX, LY, LZ}) where {LX, LY, LZ} =
ContinuousForcing{LX, LY, LZ}(on_architecture(to, forcing.func),
@@ -168,4 +168,3 @@ on_architecture(to, forcing::ContinuousForcing{LX, LY, LZ}) where {LX, LY, LZ} =
on_architecture(to, forcing.field_dependencies),
on_architecture(to, forcing.field_dependencies_indices),
on_architecture(to, forcing.field_dependencies_interp)) |
No luck with |
Hi, I'm new to Oceananigans and Julia, so I'm not sure if this is the same issue. However, I also have issues when trying to compile continuous forcing functions on the GPU. In addition to the Specifically, when I try to have forcing included with u, or v as When using a different Forcing, (such as a tidal forcing) that doesn't have a field dependency, I don't get any error. I'm using Oceananigans v0.95.20. Here is my Script:
And Here is my Error:
|
Hey @darosaj! If you can share the full error stacktrace we can confirm but it seems like the same error. Unfortunately I don't know how to fix this issue yet, but the fully working alternative is to use |
Hi @ali-ramadhan, thank you for the help! I'll use the discrete forcing in the meantime. This is the full error trace:
|
I believe the fix is on #4008 |
I'd be curious if it fixes the issue with @darosaj's code! Unfortunately it did not fix the MWE from this issue's original post: #4165 (comment) |
Seems that even simple
ContinuousForcing
functions don't want to compile on the GPU.I haven't been able to debug why this is happening but I first encountered this issue a few months ago so it's been around for some time. I couldn't really figure out what
ijl_get_nth_field_checked
does as there are very few results online.It's not the most urgent issue since
DiscreteForcing
is an option and it works. But would be nice to useContinuousForcing
so I'm opening this issue in case anyone has any ideas on what's going on.MWE:
Error:
Environment: Oceananigans v0.95.17 with
The text was updated successfully, but these errors were encountered: