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

Bump Oceananigans to 0.90 #157

Merged
merged 12 commits into from
Nov 8, 2023
Merged

Bump Oceananigans to 0.90 #157

merged 12 commits into from
Nov 8, 2023

Conversation

jagoosw
Copy link
Collaborator

@jagoosw jagoosw commented Nov 7, 2023

Some (internal) fixes required for light attenuation.

@navidcy
Copy link
Collaborator

navidcy commented Nov 7, 2023

We need to update the example in the README.md + docs landing page.

Copy link

codecov bot commented Nov 7, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (1658ddf) 64.18% compared to head (f610b33) 64.19%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #157   +/-   ##
=======================================
  Coverage   64.18%   64.19%           
=======================================
  Files          27       27           
  Lines        1061     1064    +3     
=======================================
+ Hits          681      683    +2     
- Misses        380      381    +1     
Files Coverage Δ
src/Light/2band.jl 89.18% <100.00%> (+0.61%) ⬆️
src/Models/AdvectedPopulations/LOBSTER/LOBSTER.jl 70.00% <ø> (-0.50%) ⬇️
src/Models/AdvectedPopulations/NPZD.jl 87.50% <ø> (-0.16%) ⬇️
src/Light/Light.jl 75.00% <66.66%> (-25.00%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jagoosw
Copy link
Collaborator Author

jagoosw commented Nov 8, 2023

This is now failing from a bug in Oceananigans, I'll make a PR there (CliMA/Oceananigans.jl#3383) and maybe just change how the temperature is set in this example for now (since it doesn't have any movement it can just be a function field instead).

@jagoosw
Copy link
Collaborator Author

jagoosw commented Nov 8, 2023

Actually changing this example to have a function field makes it quite messy since T is a tracer required by the bgc model so we would have to predefined all of the tracers like:

clock = Clock(; time = 0.0)
T = FunctionField{Center, Center, Center}(temp, grid; clock)
P = CenterField(grid)
N = CenterField(grid)

model = NonhydrostaticModel(..., tracers = (; N, P, T))

Which takes away from our demonstration of the automatic setup. I am going to try and run the example with a temperature forcing that is the derivative of the prescribed temperature and see if the time resolution is sufficient for it to work correctly.

@jagoosw
Copy link
Collaborator Author

jagoosw commented Nov 8, 2023

Should we merge #156 then this with a minorpatch version update, then #155 then minor release?

@navidcy
Copy link
Collaborator

navidcy commented Nov 8, 2023

We can just do a minor update when all three are merged?

@jagoosw
Copy link
Collaborator Author

jagoosw commented Nov 8, 2023

Oh yeah I meant patch but we can just merge them all and do a minor release when #155 is merged

@jagoosw jagoosw requested a review from navidcy November 8, 2023 15:44
@jagoosw jagoosw added the package label Nov 8, 2023
@jagoosw jagoosw merged commit 8d3a355 into main Nov 8, 2023
4 checks passed
@jagoosw jagoosw deleted the jsw/bump-oceananigans-0.90 branch November 8, 2023 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants