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

(0.5.0) Updated sediment to work with all grids and model types #127

Merged
merged 19 commits into from
Aug 29, 2023

Conversation

jagoosw
Copy link
Collaborator

@jagoosw jagoosw commented Aug 4, 2023

This PR makes sediment models work with immersed boundary grids and improves sediment testing on all grid and model types.

This breaks the sediment, nitrogen/carbon flux, and sinking flux APIs.

@jagoosw
Copy link
Collaborator Author

jagoosw commented Aug 4, 2023

Think this is probably the most straightforward way to set this up, and in this case the bottom cell only has to be calculated once

@jagoosw
Copy link
Collaborator Author

jagoosw commented Aug 4, 2023

I think in the same vein it may be worth modifying PAR integration to only integrate down to the bottom cell since a lot of the globe doesn't reach the bottom of the underlying grid (and at the moment I think we integrate it for the land area too)

@codecov
Copy link

codecov bot commented Aug 4, 2023

Codecov Report

Patch coverage: 76.00% and project coverage change: -0.24% ⚠️

Comparison is base (ef69e24) 63.82% compared to head (3a352ad) 63.58%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #127      +/-   ##
==========================================
- Coverage   63.82%   63.58%   -0.24%     
==========================================
  Files          28       28              
  Lines        1020     1049      +29     
==========================================
+ Hits          651      667      +16     
- Misses        369      382      +13     
Files Changed Coverage Δ
src/Light/2band.jl 88.88% <ø> (ø)
src/Light/Light.jl 33.33% <ø> (ø)
src/Models/AdvectedPopulations/NPZD.jl 90.90% <50.00%> (-1.10%) ⬇️
src/Boundaries/Sediments/Sediments.jl 65.71% <63.63%> (+6.89%) ⬆️
src/Models/AdvectedPopulations/LOBSTER/LOBSTER.jl 85.00% <66.66%> (+0.78%) ⬆️
...c/Boundaries/Sediments/instant_remineralization.jl 85.00% <100.00%> (+2.64%) ⬆️
src/Boundaries/Sediments/simple_multi_G.jl 90.38% <100.00%> (+0.80%) ⬆️

... and 1 file with indirect coverage changes

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

@jagoosw jagoosw added bug Something isn't working feature science labels Aug 10, 2023
@jagoosw
Copy link
Collaborator Author

jagoosw commented Aug 23, 2023

Since it's just the codecov failing (and I'm not bothered by the lines in the NPZD model it says aren't being tested as it's just a warning and function no one will probably ever use) I'm happy for this PR to be merged now

@jagoosw jagoosw changed the title Updated sediment to work with all grids and model types (0.5.0) Updated sediment to work with all grids and model types Aug 25, 2023
@jagoosw jagoosw removed the request for review from navidcy August 25, 2023 15:30
@jagoosw
Copy link
Collaborator Author

jagoosw commented Aug 25, 2023

The example was failing because I'd changed the sediment models to only use flat fields, but we can't output these still due to CliMA/Oceananigans.jl#2770 which I've not fixed yet

@jagoosw jagoosw requested review from navidcy, johnryantaylor and glwagner and removed request for glwagner August 26, 2023 13:35
@jagoosw
Copy link
Collaborator Author

jagoosw commented Aug 27, 2023

Nonhydrostatic + RK3 is not getting tested but it should

@jagoosw jagoosw merged commit ed7ec50 into main Aug 29, 2023
@jagoosw jagoosw deleted the jsw/sediment_with_bathymetry branch August 29, 2023 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working feature science
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant