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

ClimaNeverWorld #17

Merged
merged 43 commits into from
Apr 11, 2023
Merged

ClimaNeverWorld #17

merged 43 commits into from
Apr 11, 2023

Conversation

glwagner
Copy link
Member

@glwagner glwagner commented Mar 17, 2023

This PR implements the Neverworld in ClimaOcean.

with @simone-silvestri

Closes #20

@navidcy
Copy link
Collaborator

navidcy commented Mar 17, 2023

YEAH!

I was gonna do it tomorrow but GREAT

@glwagner
Copy link
Member Author

Ok, also with @navidcy !

@codecov
Copy link

codecov bot commented Mar 17, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.86 ⚠️

Comparison is base (2b02a17) 2.86% compared to head (5d2822c) 2.01%.

Additional details and impacted files
@@           Coverage Diff            @@
##            main     #17      +/-   ##
========================================
- Coverage   2.86%   2.01%   -0.86%     
========================================
  Files          7       9       +2     
  Lines        419     597     +178     
========================================
  Hits          12      12              
- Misses       407     585     +178     
Impacted Files Coverage Δ
src/ClimaOcean.jl 27.27% <0.00%> (-35.89%) ⬇️
src/IdealizedSimulations/neverworld_simulation.jl 0.00% <0.00%> (ø)
src/NearGlobalSimulations/NearGlobalSimulations.jl 0.00% <0.00%> (ø)
...rGlobalSimulations/one_degree_global_simulation.jl 0.00% <0.00%> (ø)
...balSimulations/quarter_degree_global_simulation.jl 0.00% <0.00%> (ø)
...obalSimulations/twelth_degree_global_simulation.jl 0.00% <0.00%> (ø)
src/VerticalGrids.jl 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

x₁, y₁ = line.p₁.x, line.p₁.y
x₂, y₂ = line.p₂.x, line.p₂.y

# Line segment lengths
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these are not any lengths of anything... it's just the differences of the line-segment's coordinates, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are components of a vector that points along the segment?

Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>
Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>
@glwagner
Copy link
Member Author

Can you look this over @navidcy or @simone-silvestri ?

@glwagner
Copy link
Member Author

Animation from the last 4 years of a 200 year "CATKE Neverworld" run with a seasonal cycle.

catke_neverworld.mp4

@navidcy
Copy link
Collaborator

navidcy commented Apr 10, 2023

I’ll have a look tomorrow!

using ParameterEstimocean
using ParameterEstimocean.Utils: map_gpus_to_ranks!
using ParameterEstimocean.Observations: FieldTimeSeriesCollector
using ParameterEstimocean.Parameters: closure_with_parameters
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we give up on the 1 degree calibration?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so --- do you want to keep maintaining it? I don't think we have plans to return to it so it makes sense to declare now that we are not maintaining it (we can always access this code in git history if we want to). What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed!

@@ -0,0 +1,43 @@
using Oceananigans
using Oceananigans.ImmersedBoundaries: mask_immersed_field!
Copy link
Collaborator

@navidcy navidcy Apr 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is mask_immersed_field! used?

using Statistics

# Some plotting parameters
ζlim = 6e-5
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ζlim = 6e-5
ζlim = 6e-5 # s⁻¹

Comment on lines 7 to 8
minimum_turbulent_kinetic_energy = 1e-6
minimum_convective_buoyancy_flux = 1e-11
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
minimum_turbulent_kinetic_energy = 1e-6
minimum_convective_buoyancy_flux = 1e-11
minimum_turbulent_kinetic_energy = 1e-6 # m² s⁻²
minimum_convective_buoyancy_flux = 1e-11 # UNITS?

spline = CubicSpline(coordinates, nodes)
S = typeof(spline)

d == :x || d == :y || d == :z || error("Dimension 'd' must be :x or :y or :z")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would that work with a LatitudeLongitude grid with λ/φ coordinates?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, here "x" etc only refers to a positional argument. The only point of using "y" is if you have a two-argument function (such as what we load into GridFittedBottom) and need to call this with (x, y), where the spline only varies in the second argument.

We could also get rid of this concept since I don't think it's needed (yet)

Comment on lines +1 to +21
# Neverworld
#
# Ingredients:
#
# * Zonally-periodic domain with continental shelves on all boundaries except Southern Ocean
# * longitude = (0, 60)
# * 2 configurations in latitude
# - Half basin: latitude = (-70, 0)
# - Full basin: latitude = (-70, 70)
# * z to -4000m
# * Southern Ocean channel from -60 to -40 (with a ridge to -2000 m)
#
# * Zonally-homogeneous wind stress with mid-latitude jet and trade winds
#
# * Buoyancy
# * restoring hot at the equator and cold at the poles (parabola, cosine, smooth step function)
# * equator-pole buoyancy differential: 0.06 (α * g * 30 ≈ 0.06 with α=2e-4, g=9.81)
# * exponential initial vertical stratification with N² = 6e-5 and decay scale h = 1000 m
# - eg bᵢ = N² * h * exp(z / h)
#
# * Quadratic bottom drag with drag_coefficient = 1e-3
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should these be part of the docstring of neverworld_simulation()?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can though I'm not a huge fan of massive docstrings, I think they are not user-friendly...

Better to put them in legit documentation which we don't have yet.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK.

Docstrings could be either user friendly or not, depending on how they are written.

As a user of any repo I feel it's really user-friendly if I do help?> a_method and I get back a useful, meaningful, docstring with some examples that make sense.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course I'm in favor of doctstrings, but I don't think we should put anything longer than a page in a docstring...

Copy link
Member Author

@glwagner glwagner Apr 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the baseline information most people need is: what are the function arguments. If concise examples can be provided then that is excellent. I'm less sure about docstrings that span multiple pages or contain theoretical background. For example we wouldn't load an entire oceananigans example into a docstring for Simulation. There's a limit beyond which the excess information is hurting not helping

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this might be useful but perhaps we should add it in the repo's wiki or something -- not in the main repo as it was!!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe better to put in Oceananigans wiki since that already has other useful stuff

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree!

@glwagner glwagner merged commit f19cc1f into main Apr 11, 2023
@navidcy navidcy deleted the ss-glw/clima-ocean-neverworld branch April 11, 2023 23:51
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

Successfully merging this pull request may close these issues.

20 year simulation of the "CATKE Neverworld" at 1/4 degree
3 participants