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

NearGlobalSimulations removed from ClimaOcean v0.2.0 and later #6

Open
vtamsitt opened this issue Apr 2, 2024 · 43 comments · May be fixed by #7
Open

NearGlobalSimulations removed from ClimaOcean v0.2.0 and later #6

vtamsitt opened this issue Apr 2, 2024 · 43 comments · May be fixed by #7

Comments

@vtamsitt
Copy link

vtamsitt commented Apr 2, 2024

This PR means GlobalOceanBioME no longer works. Is there more up-to-date ClimaOcean code we could update GlobalOceanBioME to run with instead? Or does it make sense to wait and set up to use OceanSeaIceModel?

@navidcy
Copy link
Contributor

navidcy commented Apr 2, 2024

Let me force GlobalOeanBioME to use a tag before this PR until we update it to use the latest ocean-sea ice model

@jagoosw
Copy link
Contributor

jagoosw commented Apr 2, 2024

Yeah I think it makes sense to keep using an old tag of ClimaOcean for experimenting until the new setup is ready, and hopefully it'll be ready by the time anyones ready to do a full run.

@vtamsitt
Copy link
Author

vtamsitt commented Apr 2, 2024

Makes sense, thanks

@navidcy
Copy link
Contributor

navidcy commented Apr 2, 2024

There is already a compat entry for ClimaOcean

ClimaOcean = "0.1.2"

So until we change this line, it enforces using ClimaOcean versions [0.1.2, 0.2.0) which will have the NearGlobalSimulations functionality. Right?

@navidcy
Copy link
Contributor

navidcy commented Apr 2, 2024

@vtamsitt just to confirm, did the code actually break or was your initial post here just anticipating the code breaking?

@navidcy navidcy changed the title NearGlobalSimulations removed NearGlobalSimulations removed from ClimaOcean v0.2.0 and later Apr 2, 2024
@vtamsitt
Copy link
Author

vtamsitt commented Apr 2, 2024

Yes it works when starting the GlobalOceanBioME environment with ClimaOcean 0.1.2.

@vtamsitt
Copy link
Author

vtamsitt commented Apr 3, 2024

Actually, there's an issue that I can't figure out what's going on with downloading the initial condition/bathymetry files from OceananigansArtifacts when calling one_degree_near_global_simulation.

I ran it on my laptop and it worked fine because the files were already saved in my ~/.julia/datadeps/ directory from previously running this. But when I tested on a remote machine without the data files already downloaded it didn't work, the url for the data files does not exist, and I can't seem to find any record of changes to the directory or these datafiles anywhere? I must be missing something...

@navidcy
Copy link
Contributor

navidcy commented Apr 4, 2024

Damn. I know what’s the issue… I’ll try to fix tomorrow.

@navidcy
Copy link
Contributor

navidcy commented Apr 4, 2024

cc @glwagner

@navidcy
Copy link
Contributor

navidcy commented Apr 4, 2024

@glwagner, this package was depending on the branch we deleted from OceananigansArtifacts.jl
Could we push it back? Do you have that branch locally?

@glwagner
Copy link

glwagner commented Apr 4, 2024

Oh, we messed up. This is a good lesson.

To explain: we received a notice that we were going to be charged $$ by github for hosting too many large files on OceananigansArtifacts.jl. To address that we deleted branches, but I didn't realize the branches were being used...

I don't have it locally. @simone-silvestri might.

In case restoring the files does not work, what else might we try? What exactly does GlobalOceanBioME need? We are not actually so far from having an alternative way of running global models with bulk formula (and we already have a nice way to initialize a global model from ECCO data). So it might not be much work to fix this all and move into the future by using the latest tools, rather than trying to rewind to the old global setup.

@vtamsitt
Copy link
Author

vtamsitt commented Apr 4, 2024

Ah ok, it did occur to me it might be related to too many large files. I have a local copy of these files, so could restore those?

I think what GlobalOceanBioME needs right now is some relatively well spun-up physics to use to test and optimise the BGC model(s), I think @jagoosw was planning to reach out to @simone-silvestri to see if we could get some other physics field outputs to use? @jagoosw did these old runs a while ago as a demo so it hasn't been actively developed for a while, and I've just started using it just to get up and running with the code and so we can do some tests and start looking at how the BGC models are looking. We definitely want to move this into the future with the latest tools, so working toward running this with bulk formula/initialised with ECCO as soon as that becomes available makes sense to me.

@jagoosw do you have thoughts?

@glwagner
Copy link

glwagner commented Apr 5, 2024

I have a local copy of these files, so could restore those?

This might work!

There's a couple options:

  • Upload the files back to OceananigansArtifacts.jl
  • Backport (ie develop in a new branch in ClimaOcean from the latest working version) a way for users to add the locally-available files so that the stuff in NearGlobalSimulations will succeed
  • Upload the files to a new repo and make a new branch of ClimaOcean that points to it (so we don't make OceananigansArtifacts any bigger, but still allow setups to be easily ported from machine to machine, like we previously supported)

@navidcy
Copy link
Contributor

navidcy commented Apr 5, 2024

@vtamsitt if you can upload the files somewhere and share with me (a repo or Dropbox or whatever) I can deal with it!

@JohnAllen01
Copy link

JohnAllen01 commented Apr 7, 2024

Hi GlobalOceanBioME team, sorry I am very new to this, but after talking to John Taylor, he kindly directed me to this GitHub repository. I've been trying your clone instruction to try to get the demonstration running on my own machine, however it seems that I don't have access to [email protected] ....
Cloning into 'GlobalOceanBioME'...
[email protected]: Permission denied (publickey).
fatal: Could not read from remote repository.

many thanks for any help or suggestions in advance, I can see that you are clearly working hard on some technical issues.

@johnryantaylor
Copy link
Contributor

Hi @JohnAllen01, this sounds like an issue connecting to github and not something specific to OceanBioME. There are somethings to try here: https://docs.github.com/en/authentication/troubleshooting-ssh/error-permission-denied-publickey. Personally, I use the Github desktop client (https://desktop.github.com) which shelters you from some of the intricacies of git.

@vtamsitt
Copy link
Author

vtamsitt commented Apr 8, 2024

@vtamsitt if you can upload the files somewhere and share with me (a repo or Dropbox or whatever) I can deal with it!

Thanks @navidcy! just shared the files with you via Google Drive

@navidcy
Copy link
Contributor

navidcy commented Apr 9, 2024

@vtamsitt could you try again to see if ClimaOcean can find and download the artifacts now?

@jagoosw
Copy link
Contributor

jagoosw commented Apr 9, 2024

Hi @JohnAllen01,

It will probably work if you do git clone https://github.com/OceanBioME/GlobalOceanBioME.git instead of the other clone line, it just won't let you push (it's a new security thing https://dev.to/writech/dealing-with-github-password-authentication-deprecation-58ae).

Let me know if you have any other problems!

@JohnAllen01
Copy link

Hi @jagoosw

Many thanks, the modified GitHub access worked perfectly. Now a niggle with my CUDA driver (not found), but I think we can solve that. Will definitely let you know.

Have a great weekend everyone
John

@vtamsitt
Copy link
Author

@navidcy ClimaOcean is still not finding the artifiacts with ClimaOcean v0.1.2, it's searching for the same url in OceananigansArtifacts.jl that isn't there anymore. Is there a different path now?

@navidcy
Copy link
Contributor

navidcy commented Apr 13, 2024

Actually we are having trouble -- I was under the impression I had sorted it out but I haven't! Sorry, I forgot to post here an update.

@vtamsitt
Copy link
Author

@navidcy ok no worries, I can work with my local copy of the files so am not actually limited by this currently.

@JohnAllen01
Copy link

JohnAllen01 commented Apr 25, 2024

Hi @vtamsitt,
Would you be so kind as to share the GoogleDrive missing artefacts files with me too.
many thanks
John

@vtamsitt if you can upload the files somewhere and share with me (a repo or Dropbox or whatever) I can deal with it!

Thanks @navidcy! just shared the files with you via Google Drive

@JohnAllen01
Copy link

Hi @navidcy,
Sorry, I think I may have sent this to the wrong person. Would you be so kind as to share the GoogleDrive missing artefacts files with me too.
many thanks
John

@vtamsitt if you can upload the files somewhere and share with me (a repo or Dropbox or whatever) I can deal with it!
Thanks @navidcy! just shared the files with you via Google Drive

@vtamsitt
Copy link
Author

Hi @vtamsitt, Would you be so kind as to share the GoogleDrive missing artefacts files with me too. many thanks John

@vtamsitt if you can upload the files somewhere and share with me (a repo or Dropbox or whatever) I can deal with it!

Thanks @navidcy! just shared the files with you via Google Drive

@JohnAllen01 you pinged the right person! Here's a google drive link with the files you should be able to access.

You need to create a directory on the machine you're running the code on
mkdir ~/.julia/datadeps/near_global_one_degree
and copy the files to this directory

Let me know if you get stuck or have questions.

@JohnAllen01
Copy link

Hi @vtamsitt, many thanks for the google drive link, that enabled me to get much further.

I am now stopped by the NPZD model component of the simulation as below.
My initial question would be have there been any recent changes to this NPZD model ? But then I realise I am not sure where it is pulling this model from and perhaps I am missing a link here.

any further advice would be most welcome; I know and accept this global model is very experimental, but I am keen to get it running, happy to put in some time.

best regards
John

[ Info: Building a model...
ERROR: MethodError: no method matching NutrientPhytoplanktonZooplanktonDetritus(; grid::ImmersedBoundaryGrid{…}, surface_photosynthetically_active_radiation::OneDegreeSurfacePAR{…}, scale_negatives::Bool, invalid_fill_value::Int64)

Closest candidates are:
NutrientPhytoplanktonZooplanktonDetritus(; grid, initial_photosynthetic_slope, base_maximum_growth, nutrient_half_saturation, base_respiration_rate, phyto_base_mortality_rate, maximum_grazing_rate, grazing_half_saturation, assimulation_efficiency, base_excretion_rate, zoo_base_mortality_rate, remineralization_rate, surface_photosynthetically_active_radiation, light_attenuation_model, sediment_model, sinking_speeds, open_bottom, scale_negatives, particles, modifiers) got unsupported keyword argument "invalid_fill_value"
@ OceanBioME ~/.julia/packages/OceanBioME/qrKuE/src/Models/AdvectedPopulations/NPZD.jl:159
NutrientPhytoplanktonZooplanktonDetritus(::FT, ::FT, ::FT, ::FT, ::FT, ::FT, ::FT, ::FT, ::FT, ::FT, ::FT, ::W) where {FT, W} got unsupported keyword arguments "grid", "surface_photosynthetically_active_radiation", "scale_negatives", "invalid_fill_value"
@ OceanBioME ~/.julia/packages/OceanBioME/qrKuE/src/Models/AdvectedPopulations/NPZD.jl:64

@JohnAllen01
Copy link

Hi @vtamsitt,

We removed the 'invalid_fill_value = 0' from the 'biogeochemistry_kwargs = (surface_photosynthetically_active_radiation = OneDegreeSurfacePAR(architecture), scale_negatives = true, invalid_fill_value = 0)' assignment in the model build; and now the model is running .... not sure whether this is critical

... due to finish Sunday with 16 CPUs on a MacBook Pro ... will probably be much faster once we can invoke the M3 Metal GPU ...

very best regards
John

@JohnAllen01
Copy link

... sadly it looks as though the 'invalid_fill_value = 0' may indeed be critical ... the model fell over on the tilmestep after 520.833 days having encountered a NaN in field N ...

[ Info: 520.833 days in 6.727 hours with Δt = 20 minutes
ERROR: time = 4.5012e7, iteration = 12670: NaN found in field N. Aborting simulation.
Stacktrace:
[1] error(s::String)
@ Base ./error.jl:35
[2] (::Oceananigans.Models.NaNChecker{…})(simulation::Simulation{…})
@ Oceananigans.Models ~/.julia/packages/Oceananigans/ctYdG/src/Models/nan_checker.jl:45
[3] (::Callback{…})(sim::Simulation{…})
@ Oceananigans.Simulations ~/.julia/packages/Oceananigans/ctYdG/src/Simulations/callback.jl:15
[4] time_step!(sim::Simulation{…})
@ Oceananigans.Simulations ~/.julia/packages/Oceananigans/ctYdG/src/Simulations/run.jl:143
[5] run!(sim::Simulation{…}; pickup::Bool)
@ Oceananigans.Simulations ~/.julia/packages/Oceananigans/ctYdG/src/Simulations/run.jl:97
[6] top-level scope
@ REPL[48]:1
Some type information was truncated. Use show(err) to see complete types.

@vtamsitt
Copy link
Author

vtamsitt commented May 6, 2024

@JohnAllen01 I believe 'invalid_fill_value' is a brute force way to set any NaN's that arise to 0, and yes, you will eventually throw an error when encountering a NaN without this as the global setup is not well calibrated yet. This is valid for OceanBioME v0.10.2, it used to be called 'nan_fill_value' and was changed a couple of months ago (see this commit).

Can you confirm which version of OceanBioME you are running with? You can check this by running
using Package
Pkg.status()

@JohnAllen01
Copy link

@vtamsitt ... ah, I appear to be using OceanBioME v0.10.1

Not sure why it has not 'instantiated' with the latest version, do you think this is a compatibility issue, or do I just need to somehow force this ... perhaps I need to just re-clone, although this clone only goes back to the 25th April ...

many thanks for your reply btw

(GlobalOceanBioME) pkg> status
Project GlobalOceanBioME v0.1.0
Status ~/GlobalOceanBioME/Project.toml
[79e6a3ab] Adapt v4.0.4
[0376089a] ClimaOcean v0.1.2 https://github.com/CliMA/ClimaOcean.jl.git#v0.1.2
[124859b0] DataDeps v0.7.13
[033835bb] JLD2 v0.4.46
⌃ [a49af516] OceanBioME v0.10.1
⌅ [9e8cae18] Oceananigans v0.90.11
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

@glwagner
Copy link

glwagner commented May 7, 2024

@JohnAllen01 you can try pinning a package if you don't want it to be updated: https://pkgdocs.julialang.org/v1/managing-packages/#Pinning-a-package

@vtamsitt
Copy link
Author

vtamsitt commented May 7, 2024

Actually @JohnAllen01 it looks like GlobalOceanBioME project.toml limits OceanBioME to version v0.10.1, and so that's why it has instantiated to that version. I can have a look and see if it will work if we bump up the compatability to v0.10.2.

@JohnAllen01
Copy link

Interestingly @vtamsitt, OceanBioME v0.10.1 did not like 'nan_fill_value=0' either, same error, i.e. 'got unsupported keyword argument "nan_fill_value" '

@vtamsitt
Copy link
Author

vtamsitt commented Jul 2, 2024

Revisiting this thread, with the merge of #80 it might be a good time to try updating GlobalOceanBioME to use the newer ClimaOcean OMIP setup. As far as I can tell, it should be straightforward to add biogeochemistry and kw_args passing to ocean_simulation as @jagoosw did in the old near_global_simulations. And to add an additional input for bgc boundary conditions (e.g. surface CO2 flux for DIC), or a way to include this in the exisitng boundary conditions. Any other suggestions? I can draft a PR for this.

@jagoosw
Copy link
Contributor

jagoosw commented Jul 4, 2024

I'm not sure that we really want to make any changes to ClimaOcean because what we actually need is a different version of this function:

https://github.com/CliMA/ClimaOcean.jl/blob/dc7a77515df68d886c1a070b328276492f0e2892/src/OceanSimulations/OceanSimulations.jl#L55-L120

because of the different things we might want todo to the ocean model. I'm also not sure it worthwhile trying to run online bgc models until the physics model is operational, so if you wanted to run a global offline model the setup would be quite different too.

I think there are things it would be good to implement in this repository down the line, for example, PAR loading (similar to how ClimaOcean loads the wind forcing etc.), bgc initial conditions loading, and our own version of ocean_model that setups a bgc ocean model. But this could be built on top of climaocean rather than within it.

I'm not sure what others think @johnryantaylor @navidcy?

@vtamsitt
Copy link
Author

vtamsitt commented Jul 5, 2024

Thanks @jagoosw for your response. Yeah, it does makes sense to not make any changes to the ClimaOcean code wherever possible, and build on top of it instead. I hadn't thought of just making a different version of ocean_model that sets up a bgc ocean model but that could be a good path forward.

Setting up PAR and bgc initial conditions loading is on my radar, that's something that can be worked on independently so I will see if I have time to work on this at some point.

For running an offline model, I still agree that's a good idea in the short term to evaluate the BGC model. Maybe I can follow up with @simone-silvestri and see about getting some spun up physics fields.

Curious for others thoughts too.

@glwagner
Copy link

glwagner commented Jul 7, 2024

I'm not sure that we really want to make any changes to ClimaOcean because what we actually need is a different version of this function:

https://github.com/CliMA/ClimaOcean.jl/blob/dc7a77515df68d886c1a070b328276492f0e2892/src/OceanSimulations/OceanSimulations.jl#L55-L120

The vision is for ocean_simulation is that it will provided a namelist style interface for constructing a wide variety of realistic ocean setups.

Within that vision, I think we would like to be able to use ocean_simulation to set up simulations that have biogeochemistry, too.

The only thing we need is to add a biogeochemistry kwarg to the function, right? What else would we need?

@jagoosw
Copy link
Contributor

jagoosw commented Jul 8, 2024

Okay great, it would probably also be useful to have some mechanism for passing custom boundary conditions in?

@vtamsitt
Copy link
Author

vtamsitt commented Jul 8, 2024

@glwagner that would be great. Yes a biogeochemistry kwarg and I think also biogeochemistry_kwargs needs to be a separate kwarg the way OceanBioME is set up unless the grid is already pre-defined (see here).

And do the BGC tracers needed to be added to tracers for tracer_advection?

And as @jagoosw said a way to pass BGC boundary conditions (e.g. surface CO2 flux) and ideally also custom BGC forcings.

@jagoosw
Copy link
Contributor

jagoosw commented Jul 8, 2024

We don't need the key words now because the grid is being passed in rather than made inside so we can initialise the BGC model before

@vtamsitt
Copy link
Author

@glwagner do you think we can move forward with this? Other than adding a biogeochemistry kwarg any thoughts on how best to pass BGC boundary conditions/forcings?

@glwagner
Copy link

@vtamsitt yes! Can you open an issue on ClimaOcean that describes what you would like to do? I think this will be pretty easy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants