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

Update Icepack z-biogeochemistry and z-aerosols with the MPAS-Sea ice column package #34

Draft
wants to merge 132 commits into
base: main
Choose a base branch
from

Conversation

njeffery
Copy link

@njeffery njeffery commented Mar 29, 2024

PR checklist

  • Short (1 sentence) summary of your PR:
    Updates Icepack z-bgc and z-aerosols to the latest mpas-sea ice column package version

  • Developer(s):
    Nicole Jeffery

  • Suggest PR reviewers from list in the column to the right.

  • Elizabeth and Tony

  • Please copy the PR test results link or provide a summary of testing completed below.
    https://acme-climate.atlassian.net/wiki/spaces/ICE/pages/4225073188/icepack.GMPAS.EC30to60E2r2.BGC+and+GMPAS.EC30to60E2r2

  • How much do the PR code changes differ from the unmodified code?

    • bit for bit with bgc and z-aerosols off
    • different at roundoff level
    • more substantial with bgc on
  • Does this PR create or have dependencies on CICE or any other models?

    • [ X] Yes - uses E3SM and a new interface
    • No
  • Does this PR add any new test cases?

    • Yes
    • No
  • Is the documentation being updated? ("Documentation" includes information on the wiki or in the .rst files from doc/source/, which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/.)

    • Yes
    • No, does the documentation need to be updated at a later time?
      • Yes
      • No
  • Please provide any additional information or relevant details below:

ECH edited to fix checkboxes

eclare108213 and others added 30 commits August 4, 2022 13:56
Update to Consortium Main Aug 17, 2022
Add calls to icepack_init_radiation to icepack driver
Add some SNICAR SSP table checks, aborts, etc
new namelist variables and options.

Add new namelist to icepack_in.  Add snicar and snicartest
options.

Minor updates to namelist output
Update snow table implementation and add SNICAR SSP Tables
Update ssp data for dEdd_snicar
Add additional SNICAR SSP fields for aerosols, BGC
apcraig and others added 24 commits August 20, 2023 09:39
Merge #86cae16d1b7c4 from CICE-Consortium/main Aug 19, 2023
…o cice-consortium/E3SM-icepack-initial-integration

Merge Icepack main #23b6c1272b50d42cad, includes thin ice enthalpy fix and zsalinity deprecation
…o e3sm230830

Update to Icepack Consortium #7952807, Sept 13, 2023
Merge Consortium Icepack #23b6c1272b50d4, Aug 30, 2023
Update to Consortium #795280797a Sept 13, 2023
…gration version

Some changes to z-aerosols in snow to be consistent with colpkg.
Still needs to be tested.
Adjust icepack interface for E3SM
…o upd230930

Merge Consortium main #390fb5518326217c2501dbf5 to branch, Sept 30, 2023
Update Icepack Consortium main #390fb5518326217c2501dbf5, Sept 30, 2023
Modifications are bfb in the physics.
BGC/Zaerosol changes make icepack consistent with mpas-seaice colpkg.

Still needs testing once merged with mpas-seaice interface changes.
…k' into njeffery/merge-bgc

merges icepack bgc changes with icepack-integration branch
 into njeffery/merge-bgc-consortium

Merge equivalent of #056fc4b, Oct 25, 2023 from njeffrey/Icepack/merge-bgc
Rayleigh_criteria was a field in zsalinity
Most of the updates are for use in init_zbgc which now
initializes many of the configs, indices and size declarations
for bgc

-Puts dust/BC into sea ice for thin snow layers rather than ocean
-make "puny" and "accuracy" consistent with carbon conservation check
-corrects algal parameter  f_v = 0.7854 to 0.7854_dbl_kind
-adds stationary fraction for nitrate when derived from nitrification
-corrects carbon conservation error - flux_bio should be defined with the
new brine height
-corrects algal source and sink reaction terms
-Adds DIC loss term
-updates algal_vel parameter value
-Corrects initial default values for DIC
-Corrects merging of bgc flux fields for aicen not aice_init

BFB with no BGC or aerosols
Bugfix in BGC, nonBFB.  BFB without BGC active
Bugfixes in bgc tracers
-corrected bgc ocean flux during lateral melt
-updated add_new_ice_bgc for compatibility with floe size distribution
-replace and greaty simplify adjust_tracer_profile with update_vertical_bio_tracers
-remove bgc tracers for small ice areas (corrects carbon conservation)
-pass carbon molar mass to icepack

Bugfixes in zaerosol snow tracers
-the two BGC snow layers are now fixed fractions of the snow thickness
-added missing snow bgc flux during ridging

Added diagnostics and updates to write statements

Verified carbon conservation in 20-day ocean-ice test.
nonBFB with bgc on. BFB with bgc off.
Corrects division by zero in ice_algae and add_new_ice_bgc
Remove small bgc values and track flux properly for conservation
Remove small area bgc and account for fluxes
Clean write statements

nonBGC in ice bgc
Comments out bgc aborts that could just be warnings
Fixes icepack setaborts subroutine to "add" warning strings
Fixes missing "add" to some bgc warning messages
Avoids division by small number in update vertical bio tracers

Code was seg faulting with debug_compile=true around icepack write statements
@apcraig
Copy link
Collaborator

apcraig commented Mar 29, 2024

I don't think this is being merged to the correct place, E3SM-Project/Icepack/main should probably remain synced to Consortium/Icepack/main. Should we merge this to Consortium Icepack main first? Or to a branch on E3SM/Icepack? Are there things in particular that needed to be reviewed, tested, refactored? How should we proceed, maybe we should chat sometime.

@eclare108213
Copy link
Collaborator

This PR is showing all of the commits from the last 2 years, so the base code definitely needs to be resync'ed first. Rebased? @njeffery has been developing and testing in E3SM, so it makes sense to merge there then pull back to the Consortium, although our preference is to test and merge in the Consortium then PR to E3SM. Either way, we'll need an iteration to resync the two repos, whether it's done as part of this PR or separately. @njeffery is your origin code from E3SM's fork or from the Consortium? git remote -v

@njeffery
Copy link
Author

All those commits are because I merged my branch to @apcraig squash merge (E3SM fork of Icpack commit 8fad768.

@apcraig
Copy link
Collaborator

apcraig commented Mar 29, 2024

I'm not clear on the history of the branch, but maybe it would be good to clarify and clean up if needed. I know we could create a clean branch. The best process depends how we're going to proceed.

Icepack in E3SM is a submodule. If we want these mods to be added just to the E3SM version of Icepack, we need to create a "clean" branch in E3SM/Icepack based on the current E3SM version so the E3SM submodule can be updated. We don't want to merge this to E3SM/Icepack/main (as the PR currently does). We would probably test with Consortium/Icepack/main at the same time to make sure these mods work both in E3SM and in Consortium.

Alternatively, this could be merged into the Consortium/Icepack/main then E3SM/Icepack/main could be resynced and the E3SM Icepack submodule could be updated. But E3SM would then get all the latest mods from the Consortium (although that's probably not a lot of stuff). I think that's how we "planned" to do things wrt E3SM/Consortium.

So, those seem like the two main options (push to E3SM and Consortium separately or push to Consortium then use in E3SM). In either case, I think we should test with both E3SM and within the Consortium (Icepack and CICE) before we finalize the PR. But open to other ideas about how to best proceed.

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.

4 participants