Skip to content

Conversation

@Katetc
Copy link
Collaborator

@Katetc Katetc commented Jul 7, 2021

Code to bring many hardcoded parameters to namelist options so that we can experiment with model sensitivity to them. The tag including these code changes will be used in the CAM PPE.

Note: There could be updates to the CLM tag (to conform to the CLM PPE) and to the PUMAS tag (only to improve comment documentation) before this PR goes in. These changes will likely be done Wednesday 7-7.

fixes #313

Copy link
Collaborator

@gold2718 gold2718 left a comment

Choose a reason for hiding this comment

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

The new namelist items really need some documentation (see notes). Otherwise, this looks okay.

@andrewgettelman
Copy link
Collaborator

andrewgettelman commented Jul 7, 2021 via email

@andrewgettelman
Copy link
Collaborator

andrewgettelman commented Jul 7, 2021 via email

@adamrher
Copy link
Collaborator

adamrher commented Jul 7, 2021

Hopefully I'm being helpful --I added some the units for most of what steve flagged. Also, I sound like a broken record, but the namelists cldfrc_dp1 and cldfrc_dp2 are hard coded into clubb_intr, since those namelists are linked up with macropdriver (which we don't use in cam6 physics). So a change like this needs to be added to clubb_intr:

         (-) deepcu(i,k) = max(0.0_r8,min(0.1_r8*log(1.0_r8+500.0_r8*(cmfmc(i,k+1)-cmfmc_sh(i,k+1))),0.6_r8))
         (+) deepcu(i,k) = max(0.0_r8,min(dp1*log(1.0_r8+dp2*(cmfmc(i,k+1)-cmfmc_sh(i,k+1))),0.6_r8))

with dp1 and dp2 linked up to those namelists. I have made these changes in my pending PR: #352

@gold2718
Copy link
Collaborator

gold2718 commented Jul 7, 2021

We do NOT need to document the sensitivities. We don’t know what they are: that’s why we are doing this.
Fine to document more specifically if you want.

Right. I do not expect that we can document values (yet) but we should document what changing these values changes in the computation.

… to clubb_intr for melting temp and dp1 and dp2
@Katetc
Copy link
Collaborator Author

Katetc commented Jul 7, 2021

Hi all. I made a first pass at addressing these comments, but I know there are still some more to do (especially micro_mg_cam). @andrewgettelman, yes, this does include Trude's additions of c6thl and c6thlb in clubb. @adamrher Thank you! Those units were super helpful. And I totally stole your code for dp1 and dp2 wholesale from your PR. Please have a look and make sure I've got it in there correctly, but I think that is addressed now.

@Katetc Katetc removed the request for review from andrewgettelman July 9, 2021 03:20
@cacraigucar cacraigucar added this to the CESM2.3 milestone Jul 12, 2021
@cacraigucar cacraigucar requested a review from fvitt July 12, 2021 18:03
@cacraigucar cacraigucar changed the title Support for the CAM Perturbed Parameter Ensemble (PPE) cam6_3_026: Support for the CAM Perturbed Parameter Ensemble (PPE) Jul 12, 2021
@Katetc Katetc requested review from cacraigucar and gold2718 July 12, 2021 22:53
Copy link
Collaborator

@gold2718 gold2718 left a comment

Choose a reason for hiding this comment

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

I have some questions that could result in code changes but they are not required.
I would like us to be using standard unit language so that we can use them with the CCPP. Since these changes are in XML comment blocks, it should not require re-running all the tests (just run one test or run xmllint).

Copy link
Collaborator

@nusbaume nusbaume left a comment

Choose a reason for hiding this comment

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

Looks good to me, although I do have some suggestions which will hopefully be easy to implement.

Copy link
Collaborator

@nusbaume nusbaume left a comment

Choose a reason for hiding this comment

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

Thanks for answering my questions/concerns! It looks great now, although I do have one last question.

<entry id="micro_mg_max_nicons" type="real" category="microphys"
group="micro_mg_nl" valid_values="" >
Maximum allowed ice number concentration
Default: 1.0e9
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess as an example of "we shouldn't have defaults listed in two different places", should this say 1.0e8 instead?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is inconsistent with what is in namelist_defaults.xml
Which is correct?

@fvitt fvitt removed their request for review July 14, 2021 21:02
@Katetc Katetc requested a review from gold2718 July 16, 2021 18:37
@gold2718 gold2718 requested a review from cacraigucar July 16, 2021 18:51
Copy link
Collaborator

@gold2718 gold2718 left a comment

Choose a reason for hiding this comment

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

Tentatively approving. Please make the requested very minor changes before merging (and complete tests of course).

<entry id="micro_mg_max_nicons" type="real" category="microphys"
group="micro_mg_nl" valid_values="" >
Maximum allowed ice number concentration
Default: 1.0e9
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is inconsistent with what is in namelist_defaults.xml
Which is correct?

doc/ChangeLog Outdated
Github PR URL: https://github.com/ESCOMP/CAM/pull/398

Purpose of changes (include the issue number and title text for each relevant GitHub issue):
- Adding in namelist parameters and supporting code to be used in the CAM PPE
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just needs issue number added:

Suggested change
- Adding in namelist parameters and supporting code to be used in the CAM PPE
- #313: Adding in namelist parameters and supporting code to be used in the CAM PPE

Comment on lines 73 to 77
cheyenne/intel/aux_cam:

izumi/nag/aux_cam:

izumi/pgi/aux_cam:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please fill out this report (including the expected FAIL for the DAE test).

@Katetc Katetc merged commit 3a95fb8 into ESCOMP:cam_development Jul 17, 2021
@Katetc Katetc deleted the cam_dev_ppe_aerosol_name branch November 23, 2021 22:09
cacraigucar added a commit to cacraigucar/CAM that referenced this pull request Aug 4, 2022
Merge pull request ESCOMP#398 from PUMASDevelopment/cam_dev_ppe_aerosol_name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

8 participants