OceanBioME / OceanBioME.jl

🌊 🦠 🌿 A fast and flexible modelling environment written in Julia for modelling the coupled interactions between ocean biogeochemistry, carbonate chemistry, and physics
https://oceanbiome.github.io/OceanBioME.jl/
MIT License
40 stars 20 forks source link

error in `box.jl` example #175

Closed francispoulin closed 2 months ago

francispoulin commented 4 months ago

I am keen to learn how to use this library and I decided to start by running the example.

Below you can see that I have only the barebones installed for testing purposes.

Unfortunately, I seem to get an error with Clock. Any idea what might have gone wrong here?

(@v1.10) pkg> status
Status `~/.julia/environments/v1.10/Project.toml`
  [a49af516] OceanBioME v0.10.2
⌅ [9e8cae18] Oceananigans v0.90.14
Info Packages marked with ⌅ have new versions available but compatibility constraints restrict them from upgrading. To see why use `status --outdated`

julia> include("box.jl")
ERROR: LoadError: MethodError: no method matching Clock(::Float64, ::Int64, ::Int64)

Closest candidates are:
  Clock(::TT, ::DT, ::Int64, ::Int64) where {TT, DT}
   @ Oceananigans ~/.julia/packages/Oceananigans/aI5AQ/src/TimeSteppers/clock.jl:16

Stacktrace:
 [1] top-level scope
   @ ~/Software/OceanBioME.jl/examples/box.jl:28
 [2] include(fname::String)
   @ Base.MainInclude ./client.jl:489
 [3] top-level scope
   @ REPL[18]:1
 [4] top-level scope
   @ ~/.julia/packages/CUDA/jdJ7Z/src/initialization.jl:206
in expression starting at /home/fpoulin/Software/OceanBioME.jl/examples/box.jl:28
navidcy commented 4 months ago

hm... @jagoosw was there a breaking change in Clock done in a PR in Oceananigans that you were implementing or is my memory fooling me? perhaps we need to fix the compat entries?

francispoulin commented 4 months ago

I see that clock was updated in #3508 back in March where a new argument was added to clock.

navidcy commented 4 months ago

I was under the impression that it wasn't a breaking change though... might be wrong

francispoulin commented 4 months ago

I should add that when I run it with julia with the following libraries it fails

(@v1.10) pkg> status
Status `~/.julia/environments/v1.10/Project.toml`
  [a14bc488] ArtifactWrappers v0.2.0
  [6eacf6c3] CLIMAParameters v0.9.1
⌃ [13f3f980] CairoMakie v0.11.11
  [63c18a36] KernelAbstractions v0.9.19
⌃ [85f8d34a] NCDatasets v0.13.2
  [a49af516] OceanBioME v0.10.2
⌅ [9e8cae18] Oceananigans v0.90.14
  [f27b6e38] Polynomials v4.0.8
  [1fd47b50] QuadGK v2.9.4
⌃ [49b00bb7] SurfaceFluxes v0.9.4
  [b60c26fb] Thermodynamics v0.12.6
  [de0858da] Printf
Info Packages marked with ⌃ and ⌅ have new versions available. Those with ⌃ may be upgradable, but those with ⌅ are restricted by compatibility constraints from upgrading. To see why use `status --outdated`

However, when I run it with julia --project it does run, with the following packagges

(OceanBioME) pkg> status
Project OceanBioME v0.10.2
Status `~/Software/OceanBioME.jl/Project.toml`
⌃ [79e6a3ab] Adapt v4.0.3
⌃ [052768ef] CUDA v5.2.0
⌃ [033835bb] JLD2 v0.4.46
⌃ [63c18a36] KernelAbstractions v0.9.18
⌃ [9e8cae18] Oceananigans v0.90.11
⌃ [f2b01f46] Roots v2.1.2
  [09ab397b] StructArrays v0.6.18
Info Packages marked with ⌃ have new versions available and may be upgradable.

Note that it fails with Oceananigans v0.90.14 but runs with v0.90.11.

francispoulin commented 4 months ago

Also, this shows the problem. If we add an Inf into the argument it works fine, but at the moment boxmodel.jl only has three arguments, and that produces an error, as you can see below.

I can suggest a PR if people like?

julia> using Oceananigans: Clock

julia> Clock(0.0, Inf, 0, 1)
Clock{Float64, Float64}: time = 0 seconds, last_Δt = Inf days, iteration = 0, stage = 1

julia> Clock(0.0, 0, 1)
ERROR: MethodError: no method matching Clock(::Float64, ::Int64, ::Int64)

Closest candidates are:
  Clock(::TT, ::DT, ::Int64, ::Int64) where {TT, DT}
   @ Oceananigans ~/.julia/packages/Oceananigans/aI5AQ/src/TimeSteppers/clock.jl:16

Stacktrace:
 [1] top-level scope
   @ REPL[3]:1
jagoosw commented 4 months ago

The change wasn't breaking to the initialisation function: https://github.com/CliMA/Oceananigans.jl/blob/c91df929ac50b5200b72118424db0ed3478faa9d/src/TimeSteppers/clock.jl#L32-L36 but here we directly make the struct. There is about to be another change to the clock so after that is merged I'll update OceanBioME to the latest version.

Note that it fails with Oceananigans v0.90.14 but runs with v0.90.11.

I gather from this that the Project currently has compats that it works with then right?

francispoulin commented 4 months ago

Thank you @jagoosw !

Yes, when I run OceanBIOME from the folder I cloned, it works well.

I am happy to help if I can but it sounds like you have this under control.

jagoosw commented 2 months ago

Sorry it took me a while todo but I've just updated to the latest oceananigans now