EAMxx: Adds aerosols heterogeneous freezing calculations in P3 microphysics #6947
Conversation
|
|
TODO:
|
singhbalwinder
changed the title
|
Qucik comments:
|
| const auto mask = qc_incld > qsmall; | ||
| switch (Iflag) { | ||
| case 1: // cloud droplet immersion freezing | ||
| ncheti_cnt.set(mask, frzimm*1.0e6/rho /* frzimm input is in [#/cm3] */ , Zero); |
There was a problem hiding this comment.
In all these "set" calls, how often do you expect the mask to be true/false? If the mask could often be ALL false (not sometimes, often), then you may consider using if statements, to avoid computing the packs for the true case for nothing (e.g., in the 1st line we have to compute frzimm*1e6/rho regardless of whether we need it or not).
Note: this nano-opt makes sense only if you expect mask to be often false. I assume that's not the case, since qsmall is very small. But I don't know how qc_incld is computed, so maybe it's often 0?
There was a problem hiding this comment.
That is a good question. I am not sure about that. @kaizhangpnl or @AaronDonahue might know if mask can often be false or not.
components/eamxx/src/physics/p3/tests/p3_nc_conservation_tests.cpp
Outdated
Show resolved
Hide resolved
|
Requesting reviews from @hassanbeydoun and @brhillman because I know they're very curious about and interested in this part of the p3 code |
singhbalwinder
force-pushed
the
jroverf/singhbalwinder/eamxx/add-het-frz-p3_rebase1_1
branch
from
487f9b6 to
9be599e
Compare
| add_field<Required>("hetfrz_immersion_nucleation_tend", scalar3d_layout_mid, | ||
| frz_unit, grid_name, ps); | ||
|
|
||
| // heterogeneous freezing by contact nucleation [cm^-3 s^-1] | ||
| add_field<Required>("hetfrz_contact_nucleation_tend", scalar3d_layout_mid, frz_unit, | ||
| grid_name, ps); | ||
|
|
||
| // heterogeneous freezing by deposition nucleation [cm^-3 s^-1] | ||
| add_field<Required>("hetfrz_deposition_nucleation_tend", scalar3d_layout_mid, | ||
| frz_unit, grid_name, ps); |
There was a problem hiding this comment.
@singhbalwinder this is the last potential important comment, otherwise, I think the PR is mergeable:
What happens if MAM is inactive? What happens to these variables? Maybe we should make these hidden behind if-else? IF so, make sure to do the same below (around 333):
// Inputs for the heteogeneous freezing
diag_inputs.hetfrz_immersion_nucleation_tend = get_field_in("hetfrz_immersion_nucleation_tend").get_view<const Pack**>();
diag_inputs.hetfrz_contact_nucleation_tend = get_field_in("hetfrz_contact_nucleation_tend").get_view<const Pack**>();
diag_inputs.hetfrz_deposition_nucleation_tend = get_field_in("hetfrz_deposition_nucleation_tend").get_view<const Pack**>();where in the unused/uneeded case, you can use the buffer.unused or something like that
There was a problem hiding this comment.
When MAM is inactive, values for these variables will be picked up from the namelist and will stay the same for the entire simulation. The simulation will run as usual with no impact from these variables.
I can hide it behind an if/else if it is not the desired behavior. Is there a way for P3 to know if MAM4 is active or not? Previously, when I discussed this, the design principle did not allow this, as each parameterization should be able to run independently without the knowledge of other parameterizations/processes. Otherwise, it makes the logic complex (e.g., to decide if different combinations of processes are valid or not). If it has changed, please let me know, and I will add if/else blocks.
There was a problem hiding this comment.
If they are defined, then there's no need to worry (I wasn't aware they would be defined; this is a little surprising to me tbh)
For hiding them, I would use the boolean you're adding here (hetfrz). It doesn't matter though, whether these statements are hidden or not, won't make a difference as far as I could tell. I was worried a CIME case would error out if these variables are here, but MAM is inactive. I guess we will pick this up in testing soon enough :)
There was a problem hiding this comment.
I think I would hide these behind the boolean you introduced in this PR. For the other branch of the if, set the buffer.unused or any other dummy values
There was a problem hiding this comment.
Looks good to me. If the tests pass, I support merging.
For the record, I will note that Balwinder, Luca, and I all agree we likely need to restructure the P3 code at some point in the future. This is outside the scope of the current PR, and we will think about finding the time to do it at a later point.
|
One of the public CI tests failed with (which I think is related to my comment here #6947 (comment)) to reproduce locally, this is the test: |
The heterogeneous freezing calculations from prognostics aerosols are
added to P3 microphysics. Setting
use_hetfrz_classnuctotruewill turn on these calculations. Otherwise, P3 will use the default
prescribed aerosol calculations.
[BFB] for EAM and EAMxx