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 bgc case settings documentation take 2 #4

Merged
merged 4 commits into from
Sep 11, 2024

Conversation

njeffery
Copy link

For detailed information about submitting Pull Requests (PRs) to the CICE-Consortium,
please refer to: https://github.com/CICE-Consortium/About-Us/wiki/Resource-Index#information-for-developers

PR checklist

  • [ x] Short (1 sentence) summary of your PR:
    updates to documentation of bgc namelist and env variables
  • [ x] Developer(s):
    njeffery, ehunke, apcraig
  • Suggest PR reviewers from list in the column to the right.
    apcraig
  • [x ] Please copy the PR test results link or provide a summary of testing completed below.
    visual check in html
  • How much do the PR code changes differ from the unmodified code?
    • [ x] bit for bit
    • different at roundoff level
    • more substantial
  • Does this PR create or have dependencies on CICE or any other models?
    • Yes
    • [x ] 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/.)
    • [ x] Yes
    • No, does the documentation need to be updated at a later time?
      • Yes
      • No
  • [ x] Please document the changes in detail, including why the changes are made. This will become part of the PR commit log.
    -Updates bgc environment variables
    -Corrects and updates BGC namelist parameter table
    -Corrects and updates BGC parameter arrays table

BFB

-Updates bgc environment variables
-Corrects and updates BGC namelist parameter table
-Corrects and updates BGC parameter arrays table

BFB
"``alpha2max_low_diatoms``", "real", "light limitation diatoms 1/(W/m^2)", "0.3"
"``alpha2max_low_phaeo``", "real", "light limitation phaeocystis 1/(W/m^2)", "0.17"
"``alpha2max_low_sp``", "real", "light limitation small plankton 1/(W/m^2)", "0.2"
"``ammoniumtype``", "real", "mobility type between stationary and mobile ammonium", "0.0"
"``beta2max_diatoms``", "real", "light inhibition diatoms 1/(W/m^2)", "0.001"
"``beta2max_phaeo``", "real", "light inhibition phaeocystis 1/(W/m^2)", "0.04"
"``beta2max_sp``", "real", "light inhibition small plankton 1/(W/m^2)", "0.001"
"``bgc_flux_type``", "``constant``", "constant ice–ocean flux velocity", "``Jin2006``"
"", "``Jin2006``", "ice–ocean flux velocity of :cite:`Jin06`", ""
"``bgc_data_type``", "``clim``", "bgc climatological data", "``default``"
Copy link
Owner

Choose a reason for hiding this comment

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

bgc_data_type is in forcing_nml in Icepack and CICE. Please check the definitions there and remove it here.

@njeffery
Copy link
Author

@apcraig : Okay, this last commit removes bgc_data_type. Should be good now. This field isn't used in mpas-seaice. So if the options are good for cice & icepack, then that's fine.

"``alpha2max_low_diatoms``", "real", "light limitation diatoms 1/(W/m^2)", "0.3"
"``alpha2max_low_phaeo``", "real", "light limitation phaeocystis 1/(W/m^2)", "0.17"
"``alpha2max_low_sp``", "real", "light limitation small plankton 1/(W/m^2)", "0.2"
"``ammoniumtype``", "real", "mobility type between stationary and mobile ammonium", "0.0"
"``beta2max_diatoms``", "real", "light inhibition diatoms 1/(W/m^2)", "0.001"
"``beta2max_phaeo``", "real", "light inhibition phaeocystis 1/(W/m^2)", "0.04"
"``beta2max_sp``", "real", "light inhibition small plankton 1/(W/m^2)", "0.001"
"``bgc_flux_type``", "``constant``", "constant ice–ocean flux velocity", "``Jin2006``"
"``bgc_flux_type``", "``constant``", "constant ice–ocean flux velocity", "``Jin2006``"
Copy link
Owner

Choose a reason for hiding this comment

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

looks like there is an extra space at the start of the line.

@@ -472,14 +476,14 @@ zbgc_nml
"``K_Sil_phaeo``", "real", "silicate half saturation phaeocystis mmol/m^3", "0.0"
"``K_Sil_sp``", "real", "silicate half saturation small plankton mmol/m^3", "0.0"
"``kn_bac_protein``", "real", "bacterial degradation of DON per day", "0.2"
"``l_sk``", "real", "characteristic diffusive scale in m", "20.0"
"``l_sk``", "real", "characteristic diffusive scale in m", "2.0"
Copy link
Owner

Choose a reason for hiding this comment

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

The default in Icepack and in namelist is currently 20 and we use 20. Should it be 2?

Copy link
Author

Choose a reason for hiding this comment

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

yes.

@@ -488,20 +492,20 @@ zbgc_nml
"``mu_max_sp``", "real", "maximum growth rate small plankton per day", "0.41"
"``nitratetype``", "real", "mobility type between stationary and mobile nitrate", "-1.0"
"``op_dep_min``", "real", "light attenuates for optical depths exceeding min", "0.1"
"``phi_snow``", "real", "snow porosity for brine height tracer", "-0.3"
"``phi_snow``", "real", "snow porosity for brine height tracer (compute from snow density if negative)", "-1.0"
Copy link
Owner

Choose a reason for hiding this comment

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

What is going on here. Our default is -0.3. Is it the case that any negative value has the same impact? Should we change the value to -1.0 in the code and namelist?

Copy link
Author

Choose a reason for hiding this comment

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

Any negative value does the same thing. Snow porosity is computed from snow density.

"``solve_zsal``", "logical", "DEPRECATED", "``.false.``"
"``tau_max``", "real", "long time mobile to stationary exchanges", "7776000."
"``tau_min``", "real", "rapid module to stationary exchanges", "3600."
"``tau_max``", "real", "long time mobile to stationary exchanges (s)", "604800.0"
Copy link
Owner

Choose a reason for hiding this comment

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

The Icepack default is 7776000 and that's what we run with. Is that value correct?

Copy link
Author

Choose a reason for hiding this comment

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

I've changed it to be consistent with MPAS-Si.

Copy link
Owner

Choose a reason for hiding this comment

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

We're not documenting MPAS-Si. What should the default be in Icepack? Should we be using 604800 in Icepack and CICE then? We'll need to update the code too if these defaults should change.

Copy link
Author

Choose a reason for hiding this comment

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

@apcraig : i'm making this changes because this is what I think should be in the icepack documentation. But if you'd like to keep it as is, that's fine with me.

Copy link
Owner

Choose a reason for hiding this comment

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

Happy to make the change, just want to make sure.

@apcraig apcraig merged commit 43c81a0 into apcraig:bgcpr1 Sep 11, 2024
1 check passed
@njeffery njeffery deleted the bgcpr1-nj branch September 12, 2024 14:19
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.

2 participants