CliMA / LESbrary.jl

📚Generating Oceananigans large eddy simulation (LES) data for calibrating parameterizations
MIT License
29 stars 11 forks source link

Keep Oceananigans at v0.44.0 #35

Closed ali-ramadhan closed 3 years ago

ali-ramadhan commented 3 years ago

@adelinehillier and I were having issues running the three-layer constant flux example as Oceananigans was set to #master in the Manifest.toml.

ali-ramadhan commented 3 years ago

@glwagner For context I think compute!(buoyancy) was complaining that some non-isbits argument made its way into a CUDA kernel, but wasn't sure what could have caused that in v0.44.{1,2}.

adelinehillier commented 3 years ago

I think it was specifically when we were working with 0.44.1 that the error was thrown.

glwagner commented 3 years ago

Didn't one of those versions get rid of adapt_structure for OffsetArray?

I think the problem is here:

https://github.com/CliMA/Oceananigans.jl/blob/672fa9e06aa071360c0451404a1655ba37869b30/src/Buoyancy/buoyancy_field.jl#L122

I think we just need to call adapt on data. I'm not sure why the tests don't fail.

glwagner commented 3 years ago

Wait, I'm confused. We are calling adapt on that line.

glwagner commented 3 years ago

Not sure if this is it, but I raised anyways: https://github.com/CliMA/Oceananigans.jl/issues/1233

navidcy commented 3 years ago

It's not a compat-related issue in Oceananigans.jl. But I have noticed that here https://github.com/CliMA/LESbrary.jl/blob/6cfe60cf44031f9357936058f08eed0f1bb5f9fc/Manifest.toml#L720-L723 OffsetArrays v1.3.1 is installed..

glwagner commented 3 years ago

OffsetArrays v1.3.1 is installed..

Isn't this because Oceananigans does not enforce OffsetArrays >1.4?