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

updates zaerosol in snow with mpassi #30

Conversation

njeffery
Copy link

[x] Short (1 sentence) summary of your PR:
Merge aerosol related changes from MPAS-SI into Icepack
[x] @njeffery @eclare108213
[x ] Suggest PR reviewers from list in the column to the right.

  • Please copy the PR test results link or provide a summary of testing completed below.
    ENTER INFORMATION HERE
  • How much do the PR code changes differ from the unmodified code?
    • [x ] bit for bit with zbgc off
    • different at roundoff level
    • more substantial
  • Does this PR create or have dependencies on CICE or any other models?
    • Yes
    • No
  • Does this PR add any new test cases?
    • Yes
      -[x] 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
    • [ x] No, does the documentation need to be updated at a later time?
      • Yes
      • No
  • Please provide any additional information or relevant details below:
    Replaces snow updates for vertical BGC aerosol code in Icepack with MPAS-SI version, which requires a new parameter, and updates related shortwave code from the same commit. The original aerosol tracer code is unchanged. The new parameter has yet to be implemented.

icepack_aerosol.F90

Replaces contents of subroutine update_snow_bgc. The new subroutine argument bio_index_o is commented out until the call in ice_algae.F90 is also updated.

icepack_parameters.F90

adds hs_ssl_min with a hard-coded value. Does this need to be in namelist?

icepack_shortwave.F90

updates icegrid calculation (for zbgc) as in https://github.com/E3SM-Project/E3SM/commit/e885a850330ba4cf8ebc63f82c48ecf36aef50e5

eclare108213 and others added 3 commits November 10, 2022 17:49
…gration version

Some changes to z-aerosols in snow to be consistent with colpkg.
Still needs to be tested.
@njeffery
Copy link
Author

@eclare108213 : I merged your PR with the latest Icepack integration version. It's still not tested but wanted to make it available for testing in CICE and Icepack. Let me know if I should change this PR to draft.

@eclare108213
Copy link
Collaborator

Cool. You can leave this one open (change to draft if you prefer), and I'll close the other one. I'll help after I move the constants.

@eclare108213
Copy link
Collaborator

@njeffery What's the status here? And @apcraig what's the best path forward - should we merge into the E3SM fork or into the Consortium?

@njeffery
Copy link
Author

@eclare108213 : I think it still needs to be tested in Icepack. I'm not waiting on it though. I'm continuing from here with new modifications to Icepack for zaerosols and zbgc.

@apcraig
Copy link
Collaborator

apcraig commented Sep 29, 2023

I'm open about the process. Maybe this should just be incorporated with the zaerosols and zbgc changes coming in the future? Or is the important enough to do as a separate PR?

Unless this needs to be in the E3SM PR this week, I would defer merging this for now. And in that case, I propose we merge the current E3SM Icepack to the Consortium, then deal with this PR later, either as part of the zaerosols/zbgc upgrade or as it's own PR to the Consortium Icepack at the appropriate time (probably after syncing Consortium back to E3SM).

@eclare108213
Copy link
Collaborator

I'm in favor of waiting to merge this, since we are so close to having the initial E3SM PR done. It can come in later with some of the other aero/BGC changes, or as a separate PR after the E3SM and Icepack repos are resynced.

@njeffery
Copy link
Author

That's fine by me.

@eclare108213
Copy link
Collaborator

Outdated PR. These changes will be included in the BGC PR.

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.

3 participants