-
Notifications
You must be signed in to change notification settings - Fork 237
GPU compiler error in tendency computation using DiscreteForcing
with GPU + Float32
+ immersed RectilinearGrid
#4192
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
I know it's not easy to debug this stuff and it's probably an upstream issue. So I'm just opening this issue to document the bug. It's also a pretty specific configuration so this is a low impact issue/bug. |
That's pretty interesting though. Is it fixed by using params = (
rate = 1,
u★ = 0
) ? |
Good catch! Should always be careful about types. Still getting the same error with params = (
rate = 1.0f0,
u★ = 0.0f0
) |
Is this a forcing-related issue or advection? What happens if you remove the forcing? |
Ah but actually you aren't adding |
Ah sorry for the typo. The forcing should be in there. I edited the MWE to include it. I was about to test with/without forcing. Turns out actually you don't need the forcing! This MWE without the forcing produces the same error: using Oceananigans
underlying_grid = RectilinearGrid(GPU(), Float32;
topology = (Bounded, Bounded, Bounded),
size = (10, 10, 10),
x = (0, 1),
y = (0, 1),
z = (-1, 0)
)
height = 1/5
width = 1/5
mount(x, y) = height * exp(-x^2 / 2width^2) * exp(-y^2 / 2width^2)
bottom(x, y) = -1 + mount(x, y)
grid = ImmersedBoundaryGrid(underlying_grid, GridFittedBottom(bottom))
model = NonhydrostaticModel(; grid)
simulation = Simulation(model, Δt=0.01, stop_iteration=1)
run!(simulation) |
You can also use |
I'm finding there is a type instability in div_vu; sometimes it is Float64 , other times Float32. It's unclear exactly whether this is producing the error but it seems suspicious. |
Maybe using Metal and a hydrostatic free surface model might shed light on the instability |
I tried that, but there is still promotion |
may have found the bug |
Yeah so #4193 closes this, provided that we add Oceananigans.defaults.FloatType = Float32 Another way is to specify Float32 in the advection scheme (should be -- I didn't test explicitly). In the context of #4193, the error can be reproduced by setting the default to Float64 (or leaving it alone) and manually setting the grid to Float32. So basically, sometimes promotions works (which is actually bad but does not error) and other times it throws an error (actually what we want, but it is surprising). |
I think we should document how to change number type somewhere |
Quick catch! Weird how sometimes it promotes and sometimes it errors haha. But definitely good to document how to properly change number type (probably mostly to |
Right I think we are hitting a compiler heuristic. When promotion is not completely inlined, we get an error. |
Where should we put this in the docs? |
Honestly I would advocate for a top-level page alongside the grids and fields pages. Could be called "Number type" or "Float precision"? Right now I think we just have this in the legacy docs which definitely need embellishing: https://clima.github.io/OceananigansDocumentation/stable/model_setup/number_type/ |
I was hoping we would eventually get around to adding tutorials for models. It might belong in such a page, or perhaps after it. Because then we can illustrate how it will affect all types simultaneously, not just one. that said building a tutorial for the models is a little daunting, whereas a simple page to comment on number type is easy, so maybe we should just throw it up and worry about a model tutorial in the longer run |
Although actually a tutorial for models would be lifting a lot of that existing material (eg just reorganizing it) |
@glwagner Does #4193 fix the MWE for you? I'm still getting the same error with Oceananigans v0.95.27. This more minimal MWE (as suggested above) still produces the GPU compilation error: using Oceananigans
underlying_grid = RectilinearGrid(GPU(), Float32;
topology = (Bounded, Bounded, Bounded),
size = (10, 10, 10),
x = (0, 1),
y = (0, 1),
z = (-1, 0)
)
bottom(x, y) = -0.5
grid = ImmersedBoundaryGrid(underlying_grid, GridFittedBottom(bottom))
model = NonhydrostaticModel(; grid)
simulation = Simulation(model, Δt=0.01, stop_iteration=1)
run!(simulation) |
Ah sorry! I could have been clearer. I think the correct answer is that what you have written is not supported. However, you can try this: using Oceananigans
Oceananigans.defaults.FloatType = Float32
underlying_grid = RectilinearGrid(GPU();
topology = (Bounded, Bounded, Bounded),
size = (10, 10, 10),
x = (0, 1),
y = (0, 1),
z = (-1, 0)
)
bottom(x, y) = -0.5
grid = ImmersedBoundaryGrid(underlying_grid, GridFittedBottom(bottom))
model = NonhydrostaticModel(; grid)
simulation = Simulation(model, Δt=0.01, stop_iteration=1)
run!(simulation) One can still override default FloatType for specific purposes / research, but this may cause compilation to fail. Another way to make this code to pass that avoids changing the default using Oceananigans
underlying_grid = RectilinearGrid(GPU(), Float32;
topology = (Bounded, Bounded, Bounded),
size = (10, 10, 10),
x = (0, 1),
y = (0, 1),
z = (-1, 0)
)
bottom(x, y) = -0.5
grid = ImmersedBoundaryGrid(underlying_grid, GridFittedBottom(bottom))
advection = Centered(Float32, order=2)
model = NonhydrostaticModel(; grid, advection)
simulation = Simulation(model, Δt=0.01, stop_iteration=1)
run!(simulation) I didn't test that so let me know if it works. |
Ah thanks for clarifying! I thought changing the default to I can confirm that both your examples work so I'll re-close the issue! |
Uh oh!
There was an error while loading. Please reload this page.
This might be similar to issue #4165 but this time the GPU compiler error is in the tendency computation, and specifically
div_𝐯u
.Going to the CPU, or switching to
Float64
, or toLatitudeLongitudeGrid
, or a not immersedRectilinearGrid
causes the MWE to work and not produce an error. So the error only comes up in this very specific configuration.MWE:
Error:
Environment: Oceananigans
main
branch (v0.95.23, commit40e0a8733
) withThe text was updated successfully, but these errors were encountered: