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

Sm may182021 #303

Merged
merged 242 commits into from
Jul 14, 2021
Merged

Sm may182021 #303

merged 242 commits into from
Jul 14, 2021

Conversation

SMoorthi-emc
Copy link
Contributor

Description

(Instructions: this, and all subsequent sections of text should be removed and filled in as appropriate.)
Provide a detailed description of what this PR does.
What bug does it fix, or what feature does it add?
Is a change of answers expected from this PR?

This PR removes "ncld" from the namelist, removes Interstitial variable "nncl", defines Model%ncnd via "nwat".
It also normalizes the snow over sea-ice by ice fraction both imported from the ice model.
It also updates some logic in FV3GFS_io.F90. It replaces "ncld" used in IO to "imp_physics"
Yes, this version does change the results, partly because of the above changes, and partly because of the changes in ccpp-physics.

Issue(s) addressed

Link the issues to be closed with this PR, whether in this repository, or in another repository.
(Remember, issues should always be created before starting work on a PR branch!)

  • fixes #<issue_number>
  • fixes noaa-emc/fv3atm/issues/<issue_number>

Testing

How were these changes tested?
What compilers / HPCs was it tested with?
Are the changes covered by regression tests? (If not, why? Do new tests need to be added?)
Have the ufs-weather-model regression test been run? On what platform?

  • Will the code updates change regression test baseline? If yes, why? Please show the baseline directory below.
  • Please commit the regression test log files in your ufs-weather-model branch
    Tested in the coupled mode at C384127 with 2 threads on Hera with intel compiler.
    No regression tests have been run

Dependencies

If testing this branch requires non-default branches in other repositories, list them.
Those branches should have matching names (ideally)

https://github.com/SMoorthi-emc/ccpp-physics/tree/SM_May182021

Do PRs in upstream repositories need to be merged first?
If so add the "waiting for other repos" label and list the upstream PRs

  • waiting on noaa-emc/nems/pull/<pr_number>
  • waiting on noaa-emc/fv3atm/pull/<pr_number>

No

SMoorthi-emc and others added 30 commits October 17, 2019 18:54
cplwav2atm flag for coupling wave to atm
…from the suites as this is included in GFS_surface_composites_inter_run
… reading fractional grid orography file and run as no fractional grid, and minor bug fix in physics driver related to the fractional grid - FV3GFS_io.F90 is modified to use lake fraction if it exists to distinguish lake from ocean
Update long names of hydrometeors to match the ccpp-physics change
…rve the option used in current s2s benchmarks
…ed to the mediator set to large values and over 100% sea ice set to values imported from icemodel. The mask identifying the ocean points to the mediator is correted based on ocean fraction. Updates also include name changes for the ice fields as changed by Denise Worthen. Also added an ignore_lake option to the namelist
@yangfanglin
Copy link
Collaborator

yangfanglin commented Jul 12, 2021 via email

@SMoorthi-emc
Copy link
Contributor Author

SDF names with GFS_v17 has been changed GFSv17alpha.
Thanks Dom for carefully checking.

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.

Looks good to me.

Copy link
Collaborator

@junwang-noaa junwang-noaa 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 making these changes.

@climbfuji
Copy link
Collaborator

ccpp-physics was merged, new hash is 5cf963e40f31a7be0a0ac84267f21401a78934e4

@SMoorthi-emc
Copy link
Contributor Author

SMoorthi-emc commented Jul 13, 2021 via email

@climbfuji
Copy link
Collaborator

The ccpp-physics hash is not yet correct. go to ccpp-physics, do "git remote update", then check out ncar main (git checkout WHATEVER_REMOTE_POINTS_TO_NCAR main), then go back to fv3atm and commit ccpp/physics as "Update submodule pointer for ccpp-physics" and push to github. You should have hash 5cf963e40f31a7be0a0ac84267f21401a78934e4 in ccpp/physics.

@SMoorthi-emc
Copy link
Contributor Author

SMoorthi-emc commented Jul 13, 2021 via email

@SMoorthi-emc
Copy link
Contributor Author

SMoorthi-emc commented Jul 14, 2021 via email

@climbfuji
Copy link
Collaborator

Now it is I think.

Well ... yes. But that update shows that your ccpp-physics branch wasn't up to date with main - it was missing the updates from the previous commit. Click on commit 5536416 to see those differences. If your code had been up to date, it would show zero updated files, just an update of the hash. This means that the current baselines are not valid, because the previous commit changed baselines. @junwang-noaa @DusanJovic-NOAA @DeniseWorthen

@SMoorthi-emc
Copy link
Contributor Author

SMoorthi-emc commented Jul 14, 2021 via email

@climbfuji
Copy link
Collaborator

I am not sure which of my branch you are comparing to. It should have been "SM_May182021b". Remember I had trouble reverting? Moorthi

I do. But in any case, the baselines were created with a ccpp-physics code that is not what is in main - means that any attempt to verify against those baselines will result in regression test failures.

@SMoorthi-emc
Copy link
Contributor Author

SMoorthi-emc commented Jul 14, 2021 via email

@climbfuji
Copy link
Collaborator

climbfuji commented Jul 14, 2021 via email

@SMoorthi-emc
Copy link
Contributor Author

SMoorthi-emc commented Jul 14, 2021 via email

@climbfuji
Copy link
Collaborator

I checked Jun's source directory on Dell and it has the correct version! Moorthi On Tue, Jul 13, 2021 at 8:12 PM Shrinivas Moorthi - NOAA Federal < @.***> wrote:

Hmm .. maybe you just didn't push the latest ccpp-physics merge from main to your ccpp-physics branch? Hang on, I am in a meeting now and will check thoroughly in an hour

@SMoorthi-emc
Copy link
Contributor Author

SMoorthi-emc commented Jul 14, 2021 via email

@climbfuji
Copy link
Collaborator

I think you merge the wrong one to ccpp-physics/main☺ Moorthi On Tue, Jul 13, 2021 at 8:15 PM Shrinivas Moorthi - NOAA Federal < @.***> wrote:

I merged your PR - I cannot merge branches. But chances are that your updated branch and the PR plus what is in main are identical. I will check that in an hour, if it is the case then we are good.

@SMoorthi-emc
Copy link
Contributor Author

SMoorthi-emc commented Jul 14, 2021 via email

@SMoorthi-emc
Copy link
Contributor Author

We need to use the branch SM_May182021b
Do I need to make another PR from that branch?

@climbfuji
Copy link
Collaborator

We need to use the branch SM_May182021b
Do I need to make another PR from that branch?

Just wait for a moment, please, until I get out of my meeting.

@climbfuji
Copy link
Collaborator

@SMoorthi-emc I finally got this fixed. The new hash for ccpp-physics is 719c162. This code is identical to your branch SM_May182021b.

I can you confirm that SM_May182021b was used in the regression testing, i.e. your submodule pointer for ccpp-physics pointed to the head of SM_May182021b (bc38771) before you updated it in commit 5536416.

Therefore, if you now update ccpp-physics again by checking out ncar main and then committing the submodule pointer update in fv3atm, everything will be ok and we are good to merge this PR.

@SMoorthi-emc
Copy link
Contributor Author

SMoorthi-emc commented Jul 14, 2021 via email

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.

8 participants