change default of atm_provides_lightning to FALSE -- potentially remove setting the default in CAM

ekluzek commented 3 months ago

Issue Type

Other (please describe below)

Issue Description

Since, currently CTSM can not handle lightning from CAM, the default should be off.

In the future after CTSM is able to do something with lightning it should still be FALSE, until CTSM uses it by default. And as such it probably should be CTSM that controls this setting rather than CAM. So really what should be done is to remove the code in build-namelist here...

  # for lightning flash freq to CTSM
  if ($simple_phys or $aqua_mode) {
    add_default($nl, 'atm_provides_lightning', 'val'=>'.false.');
  } else {
    add_default($nl, 'atm_provides_lightning', 'val'=>'.true.');
  }

The default in cmeps is FALSE, so not having it in the namelist will keep it as false.

What should be considered is to add a test (or tests) to run with it on, so that the code in CAM to do this isn't broken.

Below on answer changes testing will show a change to answers in the fieldlist of the cpl history files. But, otherwise it'll be bit-for-bit.

Summary:

[ ] Remove setting of atm_provides_lightning in build-namelist
[ ] Add at least a test turning it on
[ ] Will show a change in answers for testing compared to baseline because of a fieldlist different in the cpl history files

Will this change answers?

No

Will you be implementing this yourself?

Any CAM SE can do this

brianpm commented 2 months ago

Hi @ekluzek -- We started to talk about this issue, but have a few basic questions.

First, we were curious whether the current behavior has any adverse effects? That is, if CTSM gets lightning but doesn't use it, does that matter?

Second, you suggested that CTSM should be controlling whether it gets passed from CAM to CTSM... does this flag control anything about how CAM deals with lightning calculations, or is it totally independent?

Finally, how should we prioritize this? Does it need to be done for CESM3?

Thanks!

EDIT: Just saw this issue on CTSM, which seems highly related: https://github.com/ESCOMP/CTSM/issues/1630

ekluzek commented 2 months ago

Hi @brianpm thanks for talking about this. First, I added it because I added the ability to read in the namelist on the CTSM side, and realized this means that a user can set it and think it's doing something when it really isn't. So no there are no adverse effects -- other than the expectation that turning it on would do something. And as such it's a nice code cleanup/usability improvement -- but not absolutely required.

Second, I think CTSM should control this -- because I think the only purpose for this field is whether it's passed to CTSM or not. I agree that CAM should have control over how it's calculated. But, since CTSM won't always be using it -- CTSM should decide if it's passed or not. So as I understand the field from skimming CAM code -- it's totally independent of HOW CAM calculates lightning.

On priority, I don't have access to changing labels. I'd give it "low priority" in the CTSM repository for example. It's only important from a usability context, so not critical to do. But, still usability is helpful to work on for the release, so I'd like it to be done before the final release of CESM3.0 -- but not a "must have".

Finally, on the issue you post above, yes that is the plan for taking advantage of lightning on the CTSM side, with just this part for CAM. We do NOT plan on having this in CTSM for CESM3.0, but hope to work on it afterwards. But, that's also why I'd like it off for the release as otherwise it's a new user-facing field that doesn't actually do anything. But, the fact that it's new would suggest that it is something you could use. As a user -- I'd rather know this kind of thing "don't turn this knob, it won't hurt anything -- but it doesn't do anything either". As you can waste a lot of time trying something out that does nothing.

ESCOMP / CAM