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

Updates for P8 #1353

Merged
merged 23 commits into from
Aug 3, 2022
Merged

Conversation

JessicaMeixner-NOAA
Copy link
Collaborator

@JessicaMeixner-NOAA JessicaMeixner-NOAA commented Jul 27, 2022

PR Checklist

  • This PR is up-to-date with the top of all sub-component repositories except for those sub-components which are the subject of this PR. Please consult the ufs-weather-model wiki if you are unsure how to do this.

  • This PR has been tested using a branch which is up-to-date with the top of all sub-component repositories except for those sub-components which are the subject of this PR

  • An Issue describing the work contained in this PR has been created either in the subcomponent(s) or in the ufs-weather-model. The Issue should be created in the repository that is most relevant to the changes in contained in the PR. The Issue and the dependent sub-component PR
    are specified below.

  • Results for one or more of the regression tests change and the reasons for the changes are understood and explained below.

  • New or updated input data is required by this PR. If checked, please work with the code managers to update input data sets on all platforms.

Instructions: All subsequent sections of text should be filled in as appropriate.

The information provided below allows the code managers to understand the changes relevant to this PR, whether those changes are in the ufs-weather-model repository or in a subcomponent repository. Ufs-weather-model code managers will use the information provided to add any applicable labels, assign reviewers and place it in the Commit Queue. Once the PR is in the Commit Queue, it is the PR owner's responsibility to keep the PR up-to-date with the develop branch of ufs-weather-model.

Description

This PR updates the following:

  • This reverts the calculation of T2m in NOAH-MP as it was found in p8c that it tends to result in overly low t2min and overly high t2max values. (CCPP PR)
  • Sets do_gsl_drag_tofd=false as this solved the stability issues seen when first trying to run P8c
  • Updates to aerosol configuration:
    --- Update the RT to mimic the p8 aerosol configuration. Updates the wet deposition coefficient for organic carbon from 0.3 -> 0.4. Updates the dust to use the 10km file that is used within P8c and p8

File to be copied in RT data folder is on hera:
/scratch1/NCEPDEV/global/glopara/data/gocart_emissions/Dust/FENGSHA_p81_10km_inputs.nc
and should be added to :
/scratch1/NCEPDEV/nems/emc.nemspara/RT/NEMSfv3gfs/input-data-20220414/GOCART/p8/ExtData/dust/

Issue(s) addressed

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 regression tests and unit tests (utests) been run? On which platforms and with which compilers? (Note that unit tests can only be run on tier-1 platforms)

  • hera.intel
  • hera.gnu
  • orion.intel
  • cheyenne.intel
  • cheyenne.gnu
  • gaea.intel
  • jet.intel
  • wcoss2.intel
  • acorn.intel
  • opnReqTest for newly added/changed feature
  • CI

Dependencies

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

Do PRs in upstream repositories need to be merged first? Yes

@JessicaMeixner-NOAA JessicaMeixner-NOAA marked this pull request as ready for review July 28, 2022 20:09
@JessicaMeixner-NOAA
Copy link
Collaborator Author

@jkbk2004 I have updated the PR with Barry's updates. There is one new input file which I copied to the location on hera but not any other machine. The PR was updated to indicate it needs the new input data.

@jkbk2004
Copy link
Collaborator

@jkbk2004 I have updated the PR with Barry's updates. There is one new input file which I copied to the location on hera but not any other machine. The PR was updated to indicate it needs the new input data.

Sure! If orion comes back this evening or tomorrow, we may start testing the PR tomorrow or Monday. We are currently working on #1317 now.

@BrianCurtis-NOAA BrianCurtis-NOAA added Baseline Updates Current baselines will be updated. New Input Data Req'd This PR requires new data to be sync across platforms labels Jul 29, 2022
@JessicaMeixner-NOAA
Copy link
Collaborator Author

@HelinWei-NOAA have you run the regtests with your ccpp-physics updates? I'm having issues with the compile for:
COMPILE | -DAPP=ATM -DDEBUG=ON -D32BIT=ON | | fv3 |

for example I get this error:

/scratch1/NCEPDEV/stmp2/Jessica.Meixner/FV3_RT/rt_175447/compile_001/build_fv3_001/FV3/ccpp/physics/ccpp_static_api.F90(5012): error #6405: The same named entity from different modules and/or program units cannot be referenced.   [CDATA]
               ierr = FV3_GFS_v16_coupled_p8_sfcocn_time_vary_tsfinal_cap(cdata=cdata)
--------------------------------------------------------------------------------^
compilation aborted for /scratch1/NCEPDEV/stmp2/Jessica.Meixner/FV3_RT/rt_175447/compile_001/build_fv3_001/FV3/ccpp/physics/ccpp_static_api.F90 (code 1)
make[2]: *** [FV3/ccpp/CMakeFiles/fv3ccpp.dir/physics/ccpp_static_api.F90.o] Error 1
make[1]: *** [FV3/ccpp/CMakeFiles/fv3ccpp.dir/all] Error 2

@JessicaMeixner-NOAA
Copy link
Collaborator Author

I got the same error above when I ran from the develop branch, so perhaps this is just some weird user error on my end?

@jkbk2004
Copy link
Collaborator

I got the same error above when I ran from the develop branch, so perhaps this is just some weird user error on my end?

@JessicaMeixner-NOAA let me check on cheyenne.

@jkbk2004
Copy link
Collaborator

I got the same error above when I ran from the develop branch, so perhaps this is just some weird user error on my end?

@JessicaMeixner-NOAA let me check on cheyenne.
Both develop and this branches build ok.

@JessicaMeixner-NOAA
Copy link
Collaborator Author

@jkbk2004 thank you for the sanity check. I guess I did something on my end. Glad things are working. FYI @HelinWei-NOAA

@DeniseWorthen
Copy link
Collaborator

@JessicaMeixner-NOAA Are you sure the bmark_p8 oRT test ran? The log you posted appears empty.

@JessicaMeixner-NOAA
Copy link
Collaborator Author

@JessicaMeixner-NOAA Are you sure the bmark_p8 oRT test ran? The log you posted appears empty.

Today is not my day for running tests apparently... Let me try this again...

@jkbk2004
Copy link
Collaborator

@JessicaMeixner-NOAA Tests on orion passed.

@junwang-noaa
Copy link
Collaborator

@bbakernoaa I saw all the cpld tests have reduced >10% run time and >30% memory usage. May I ask how much resolution change is in the new dust mission file compared to previous emission files FENGSHA_SOILGRIDS2019_GEFSv12_v1.2.nc/FENGSHA_Albedo_drag_v1.nc/randomforestensemble_uthres.nc? Thank you!

@BrianCurtis-NOAA
Copy link
Collaborator

as of 10PM, Acorn isn't wanting to run RT's. I'll talk to Dusan to see how he ran them previously. Skip for this PR.

@jkbk2004
Copy link
Collaborator

jkbk2004 commented Aug 2, 2022

@JessicaMeixner-NOAA We can start merging process. I will go to fv3 pr to ask for final review and merge there. @DusanJovic-NOAA @DeniseWorthen Please, go ahead for final review.

@DeniseWorthen
Copy link
Collaborator

It doesn't look like the dcp oRT was run for the cpld cases.

@JessicaMeixner-NOAA
Copy link
Collaborator Author

It doesn't look like the dcp oRT was run for the cpld cases.

On second look, this seems correct. This was an oversight on my part. I can run this as soon as hera is back online. In the future, it would be helpful to have clearer instructions on what specific tests with what specific options are required to avoid mistakes like this in the future.

@jkbk2004
Copy link
Collaborator

jkbk2004 commented Aug 2, 2022

@JessicaMeixner-NOAA @DeniseWorthen I was asking to check ORTs of this PR on EPIC side. I will make sure. I think he was testing on Orion.

@BrianCurtis-NOAA
Copy link
Collaborator

It doesn't look like the dcp oRT was run for the cpld cases.

On second look, this seems correct. This was an oversight on my part. I can run this as soon as hera is back online. In the future, it would be helpful to have clearer instructions on what specific tests with what specific options are required to avoid mistakes like this in the future.

I want to second that there is no documentation on what needs to be run for ORT and it would be helpful to component devs like Jessica to have such a document. We call ORT's unit tests for some reason on the Wiki (https://github.com/ufs-community/ufs-weather-model/wiki/Running-unit-tests-using-utest) so that documentation needs to change and a section to be added on what ORT's need to be run for each type of test (regional, global, etc..) I've created an issue here: #1363

@jkbk2004
Copy link
Collaborator

jkbk2004 commented Aug 2, 2022

It doesn't look like the dcp oRT was run for the cpld cases.

On second look, this seems correct. This was an oversight on my part. I can run this as soon as hera is back online. In the future, it would be helpful to have clearer instructions on what specific tests with what specific options are required to avoid mistakes like this in the future.

I want to second that there is no documentation on what needs to be run for ORT and it would be helpful to component devs like Jessica to have such a document. We call ORT's unit tests for some reason on the Wiki (https://github.com/ufs-community/ufs-weather-model/wiki/Running-unit-tests-using-utest) so that documentation needs to change and a section to be added on what ORT's need to be run for each type of test (regional, global, etc..) I've created an issue here: #1363

I am asking Zach/EPIC to follow up ORT runs for new PRs until we have a good solution with Jenkins option.

@jkbk2004
Copy link
Collaborator

jkbk2004 commented Aug 2, 2022

It doesn't look like the dcp oRT was run for the cpld cases.

On second look, this seems correct. This was an oversight on my part. I can run this as soon as hera is back online. In the future, it would be helpful to have clearer instructions on what specific tests with what specific options are required to avoid mistakes like this in the future.

Can you remind me about the point? cpld_bmark_p8_dcp ran, right?

@JessicaMeixner-NOAA
Copy link
Collaborator Author

@jkbk2004 for the ort for cpld_bmark_p8 dcp did run. But for cpld_control_ciceC_p8 and cpld_control_p8 I unintentionally left out dcp

@JessicaMeixner-NOAA
Copy link
Collaborator Author

The oRT dcp test has now been run for cpld_control_ciceC_p8 and cpld_control_p8 and the log files have been updated. @jkbk2004 @DeniseWorthen

@jkbk2004
Copy link
Collaborator

jkbk2004 commented Aug 3, 2022

The oRT dcp test has now been run for cpld_control_ciceC_p8 and cpld_control_p8 and the log files have been updated. @jkbk2004 @DeniseWorthen

@JessicaMeixner-NOAA Sure! I merged in fv3atm#565. Can you update submodule pointer and revert branch in gitmodules?

@JessicaMeixner-NOAA
Copy link
Collaborator Author

@jkbk2004 FV3 has been updated

@jkbk2004 jkbk2004 merged commit 9ae617e into ufs-community:develop Aug 3, 2022
@JessicaMeixner-NOAA
Copy link
Collaborator Author

@DeniseWorthen @jkbk2004 apologies for my oversights and thank you for your patience in working with me to get this committed! It is much appreciated. This is the anticipated "Prototype8" hash now. Would it be possible to get a tag for this?

@DeniseWorthen
Copy link
Collaborator

Thanks @JessicaMeixner-NOAA. I've created a Prototype-P8 tag.

@JessicaMeixner-NOAA
Copy link
Collaborator Author

Thanks @DeniseWorthen !!!

@JessicaMeixner-NOAA JessicaMeixner-NOAA deleted the feature/p8 branch March 14, 2023 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Baseline Updates Current baselines will be updated. New Input Data Req'd This PR requires new data to be sync across platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Updates for P8
7 participants