Skip to content
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

error in box.jl example #175

Closed
francispoulin opened this issue May 13, 2024 · 8 comments
Closed

error in box.jl example #175

francispoulin opened this issue May 13, 2024 · 8 comments

Comments

@francispoulin
Copy link

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
Copy link
Collaborator

navidcy commented May 13, 2024

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
Copy link
Author

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

@navidcy
Copy link
Collaborator

navidcy commented May 13, 2024

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

@francispoulin
Copy link
Author

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
Copy link
Author

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
Copy link
Collaborator

jagoosw commented May 14, 2024

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
Copy link
Author

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
Copy link
Collaborator

jagoosw commented Jul 1, 2024

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

@jagoosw jagoosw closed this as completed Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants