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

Adds a flag to control vertical mixing of interstitial aerosols during aerosol activation process #6926

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

singhbalwinder
Copy link
Contributor

Currently, the interstitial aerosols and liquid number are mixed twice-
once by SHOC and then by the aerosol activation process. A flag is
added to control the mixing of interstitial aerosols and liquid number (nc)
by the activation process. It is currently set to true to keep the default
behavior (i.e. aerosols mixed twice). This flag will be set to false in a follow-up
PR.

[BFB]

@bartgol
Copy link
Contributor

bartgol commented Jan 22, 2025

@mahf708 do you have any idea why the gh-pages action fails on this PR, while it passes anywhere else? This PR does not touch docs, so I would expect the action to succeed...

@mahf708
Copy link
Contributor

mahf708 commented Jan 22, 2025

@mahf708 do you have any idea why the gh-pages action fails on this PR, while it passes anywhere else? This PR does not touch docs, so I would expect the action to succeed...

The PR does touch docs (the namelist stuff is part of docs technically). It doesn't produce docs preview on the PR because the job fails. (See below why.)

It fails because of the citation stuff (not related to this PR). In the logs, you see something like:

WARNING -  Affixes not supported in simple mode: @wing_rcemip1_2018 @wing_rcemip2_2024

and because we are doing this in strict mode, all warnings become fails (which is good, because these warnings are likely gonna make the docs look pretty odd).

So... why are these failing out of nowhere? There was an update in mkdocs-bibtext recently (I got a notification because I take care of the conda-forge packages for this, e.g., this). My guess is that one of these two commits broke mkdocs-bibtex ... (we should either go back to mkdocs-bibtex 2.21.0 or investigate further)

Note that I wasn't the one who introduced mkdocs-bibtex; I was mildly against it at the time, but I didn't speak up. I don't like these types of utilities/plugins in general...

@mahf708
Copy link
Contributor

mahf708 commented Jan 22, 2025

Are you expecting the np2 vs np4 type of test to fail here? Many are failing?

Comment on lines +794 to +799
for (int imode = 0; imode < num_aero_modes(); ++imode) {
dry_aero.int_aero_nmr[imode](icol,klev) = haero::max(dry_aero.int_aero_nmr[imode](icol,klev), INTERSTITIAL_AERO_MIN_VAL);
for (int ispec = 0; ispec < num_aero_species(); ++ispec) {
if (dry_aero.int_aero_mmr[imode][ispec].data()) {
dry_aero.int_aero_mmr[imode][ispec](icol,klev) = haero::max(dry_aero.int_aero_mmr[imode][ispec](icol,klev), INTERSTITIAL_AERO_MIN_VAL);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like set_min_background_mmr is unrelated to stuff in the PR title and description. I am concerned about this because it is surprisingly complex (pretty hard to read):

  • why do think this minimum is needed? just matching stuff to old mam or is there some undesirable behavior?
  • what are the examples when this dry_aero.int_aero_mmr[imode][ispec].data() is true/false? do we have any mode with no species in it?
  • relatedly, what does dry_aero.int_aero_nmr[imode](icol,klev) access when dry_aero.int_aero_mmr[imode][ispec].data() is true?

I guess I am confused about both the data layout as well as any undesirable side effect of low values...

Copy link
Contributor Author

@singhbalwinder singhbalwinder Jan 22, 2025

Choose a reason for hiding this comment

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

When enable_aero_vertical_mix == false, the model crashes because the ACI codes (specifically the heterogeneous freezing code) expect positive mmrs for the interstitial aerosols. Therefore, I added this limiter to ensure that the interstitial aerosols are always positive.

num_aero_species() is the number of species for the mam mode that has the maximum species. For example, mode 1 has the maximum species (7), but mode 2 has 4 species. When we loop over num_aero_species() (i.e., 7) for mode 2, dry_aero.int_aero_mmr[imode][ispec].data() is false for 4th, 5th, and 6th (c-indexing)iterations.

dry_aero.int_aero_nmr[imode](icol,klev) is the interstitial aerosol number mixing ratio (represented by _nmr) of mode imode at colum icol and level klev.

I hope it makes sense, but let me know if you have questions.

ekat::subview(wsub, icol), // in
ekat::subview(cloud_frac_prev, icol), // in
qqcw_view, // inout
local_enable_aero_vertical_mix, ekat::subview(qcld, icol), // out
Copy link
Contributor

Choose a reason for hiding this comment

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

is local_enable_aero_vertical_mix out? I thought it would be in...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. It should be in, I will fix this.

@@ -251,6 +251,7 @@ be lost if SCREAM_HACK_XML is not enabled.
<!-- MAM4xx-ACI -->
<mam4_aci inherit="atm_proc_base">
<wsubmin type="real" doc="Minimum diagnostic sub-grid vertical velocity">0.001</wsubmin>
<enable_aero_vertical_mix type="logical" doc="Enable vertical mixing of interstitial aerosols and liquid number during activation">true</enable_aero_vertical_mix>
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this flag is to control the vertical mixing calculation of both aerosols and cloud droplet (?) in aci, it might be better change it to "enable_aci_vert_mix". Do we have a similar flag to control the vertical mixing calculation in SHOC?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good suggestion. Thanks. I will revise the flag name.

@tcclevenger is working on a method to disable the vertical mixing in SHOC. Once that is ready, we will have two ways to control vertical mixing.

@singhbalwinder
Copy link
Contributor Author

@mahf708 : Thanks for looking at the test results. I do not expect np2 vs np4 tests to fail. I will look into it.

Also, I am hoping that this PR is BFB, but it can be NBFB due to the limiter I added to set background mmr when it is zero or negative.

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.

5 participants