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

Bugfix for levr<levs, update of gcycle.F90/sfcsub.F for coupled model (Sm mar032020) #404

Closed
wants to merge 6 commits into from

Conversation

SMoorthi-emc
Copy link
Collaborator

Hi Dom et. al,
I fixed a few bugs in the ".meta" files that would cause crash when "levr < levs".
This fix is needed for the Whole Atmosphere Model, where levs=149 but levr could be ~ 75.
While I haven't tried to run the model with 149 layers, I tried running 64 layer model with levr=63 and it crashed and that led to this fix. It should have no impact when levr=levs.
Thanks
Moorthi

Copy link
Collaborator

@climbfuji climbfuji left a comment

Choose a reason for hiding this comment

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

This all makes sense to me, as far as I can tell (I don't understand every change you are making in sfcsub.F). But I am wondering if we could skip the change of standard names (time_step -> timestep). Otherwise we'll have to make the same change in FV3 as well. I'd also like to have @grantfirl's opinion, because he is the master in composing standard names.

@SMoorthi-emc
Copy link
Collaborator Author

Hi Dom,
I don't think I added any "time-step" or "timestep" this time. You are right that there are lots of places both types are used. It would be good to standardize, but that means lots of edits. The main fix I made is to use the correct radiative heating arrays as the original code used the wrong ones (which was causing the model blowup when levr < levs)
I did not change meta files for NN (it may not matter as they may not use heating above levr).
Moorthi

@grantfirl
Copy link
Collaborator

This all makes sense to me, as far as I can tell (I don't understand every change you are making in sfcsub.F). But I am wondering if we could skip the change of standard names (time_step -> timestep). Otherwise we'll have to make the same change in FV3 as well. I'd also like to have @grantfirl's opinion, because he is the master in composing standard names.

To chime in here, Moorthi, we have been using CF conventions whenever possible: http://cfconventions.org/Data/cf-standard-names/72/build/cf-standard-name-table.html

However, there is no variable with either "time step", "time-step", or "timestep" included in those conventions. This is also unfortunately one of those words that I don't think has been officially made into a compound word yet (most dictionaries don't have an entry for it), but it is just a matter of time, IMO. So, it is up to us to decide what we want to do and we should just be consistent across the CCPP. My vote is for "timestep" since every one in the modeling community will know what we mean and it has one less character.

There is a ton of cleanup that could be done with CCPP standard names, and this is definitely something that is contributing toward user-unfriendliness at the moment. We have vague plans to do such a cleanup and to provide a search tool and "official" standard name guidelines for new variables for users, although I'm not sure where this task falls on the priority list right now.

@SMoorthi-emc
Copy link
Collaborator Author

SMoorthi-emc commented Mar 11, 2020 via email

@climbfuji
Copy link
Collaborator

@SMoorthi-emc just to make sure, there is no change needed on the fv3atm side (or any other submodule/repository)? Thanks!

@SMoorthi-emc
Copy link
Collaborator Author

You are correct.
However, if you are taking the sfcsub change, there is corresponding version for IPD, which I am not pushing at this time.

@climbfuji
Copy link
Collaborator

You are correct.
However, if you are taking the sfcsub change, there is corresponding version for IPD, which I am not pushing at this time.

This is good enough, thanks. We'll drop the IPD-CCPP REPRO comparison then.

@climbfuji climbfuji changed the base branch from master to dtc/develop March 16, 2020 15:03
@climbfuji climbfuji changed the title Sm mar032020 Bugfix for levr<levs, update of gcycle.F90/sfcsub.F for coupled model (Sm mar032020) Mar 16, 2020
@climbfuji
Copy link
Collaborator

It was decided to hold back on the sfcsub.F/gcycle.F90 updates for now.

The levr bugfix proposed here is superseded by commit c8a345a as part of PR #410.

Closing this PR.

@climbfuji climbfuji closed this Mar 17, 2020
hannahcbarnes pushed a commit to hannahcbarnes/ccpp-physics that referenced this pull request Aug 3, 2022
* remove unused coupled suites
* delete v15p2 coupled suites
* update filename when the output time is not integer hours
* fix the diag time issue with output_fh

Co-authored-by: Denise Worthen <denise.worthen@noaa.gov>
Co-authored-by: Jun Wang <junwang-noaa@users.noreply.github.com>
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.

3 participants