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

Smoke/Dust updates for RRFS code freeze #2024

Merged
merged 15 commits into from
Dec 28, 2023

Conversation

haiqinli
Copy link
Contributor

@haiqinli haiqinli commented Dec 3, 2023

PR Author Checklist:

  • I have linked PR's from all sub-components involved in section below.
  • I am confirming reviews are completed in ALL sub-component PR's.
  • I have run the full RT suite on either Hera/Cheyenne AND have attached the log to this PR below this line:
    • LOG:
  • I have added the list of all failed regression tests to "Anticipated changes" section.
  • I have filled out all sections of the template.

Description

  1. Include convective transport and wet removal of smoke/dust in the GF convection.
  2. A new dry deposition option for smoke/dust.
  3. Update the diurnal cycle of biomass burning emissions.
  4. Clean the smoke code.

Commit Message

This smoke/dust update PR is for the RRFS code freeze.

Linked Issues and Pull Requests

Associated UFSWM Issue to close

Subcomponent Pull Requests

Blocking Dependencies

"None"

Subcomponents involved:

  • AQM
  • CDEPS
  • CICE
  • CMEPS
  • CMakeModules
  • FV3
  • GOCART
  • HYCOM
  • MOM6
  • NOAHMP
  • WW3
  • stochastic_physics
  • none

Anticipated Changes

Input data

  • No changes are expected to input data.
  • Changes are expected to input data:
    • New input data.
    • Updated input data.

Regression Tests:

  • No changes are expected to any regression test.
  • Changes are expected to the following tests:
    FAILED TESTS:
    conus13km_control_intel
    conus13km_debug_intel
    conus13km_debug_qr_intel
    conus13km_debug_2threads_intel
    conus13km_radar_tten_debug_intel
    conus13km_control_gnu
    conus13km_debug_gnu
    conus13km_debug_qr_gnu
    conus13km_debug_2threads_gnu
    conus13km_radar_tten_debug_gnu
Tests effected by changes in this PR: The above RT's will be changed by the code updates. Since the inclusion of convective transport and wet removal of smoke/dust; a new dry deposition; and updated smoke emission inputs. The log file of regression test run is attached. [RegressionTests_hera.log](https://github.com/ufs-community/ufs-weather-model/files/13540338/RegressionTests_hera.log)

Libraries

  • Not Needed
  • Needed
    • Create separate issue in JCSDA/spack-stack asking for update to library. Include library name, library version.
    • Add issue link from JCSDA/spack-stack following this item
Code Managers Log
  • 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.
  • Move new/updated input data on RDHPCS Hera and propagate input data changes to all supported systems.
    • N/A

Testing Log:

  • RDHPCS
    • Hera
    • Orion
    • Hercules
    • Jet
    • Gaea
    • Cheyenne
  • WCOSS2
    • Dogwood/Cactus
    • Acorn
  • CI
    • Completed
  • opnReqTest
    • N/A
    • Log attached to comment

@DeniseWorthen
Copy link
Collaborator

DeniseWorthen commented Dec 4, 2023

@haiqinli Please check the code for errors. Your Hera log indicates the listed tests failed to run, not that the results did not compare.

@haiqinli
Copy link
Contributor Author

haiqinli commented Dec 4, 2023

@haiqinli Please check the code for errors. Your Hera log indicates the listed tests failed to run, not that the results did not compare.

Thanks for pointing this out. We introduced a new namelist option "ebb_dcycle" to support fire emission for different mode. ebb_dcycle=1 is the original option (we call it retro run mode) to read hourly emission and fire radiation power, and corresponds to the fire emission input data in the regression test run case. ebb_dcycle=2 is the new option (we call it forecast mode) to read daily mean fire emission, fire radiation power, fire duration and fire potential, and calculate the diurnal cycle of biomass burning emission within RRFS-SD. ebb_dcycle=2 will be used for RRFS-A parallel run, and was set as default configuration in the code. Since ebb_dcyle is not available in the input namelist of the regression test run cases, and the fire emission input data in the regression test run cases is for ebb_dcycle=1. Thus those regression test runs failed. I will update the code to use ebb_dcycle=1 as default configuration, and rerun the regression test. Will update the log file when the regression test rerun is done. Thanks.

@DeniseWorthen
Copy link
Collaborator

@haiqinli Thanks. You also note that new input-data is required. Is that correct? If yes, please provide the location where the data is currently staged so that code managers can transfer it to all the platforms.

You indicate this is for a code freeze. Do you have date when this will be required? We need to know if this is a high priority PR.

@haiqinli
Copy link
Contributor Author

haiqinli commented Dec 4, 2023

@DeniseWorthen Yes, we need to update the fire emission input data. We are processing a real case for conus13km, and will update when we make it done. Yes, this PR is for the RRFSv1 code freeze. We will also update when we get the exact required date. Thanks.

@DeniseWorthen
Copy link
Collaborator

@haiqinli We're adding a Commit Message requirement to the PRs. Please add one in the space provided.

@haiqinli
Copy link
Contributor Author

@DeniseWorthen We have updated the PR to address UFS code reviewers's comments, and rerun the regression test run. If we run the forecast mode for diurnal cycle of plume, the vegetation fraction from RUC LSM is required, but it is not available in the conus13km regression test cases. So I set the retro mode with hourly fire emission as default for the conus13km regression test cases, and we don't need to update the input files. The log file of regression test run is attached. Thanks.
RegressionTests_hera.log

@DeniseWorthen
Copy link
Collaborator

@haiqinli Thanks. Do you have a target date this is required by?

@haiqinli
Copy link
Contributor Author

@DeniseWorthen It would be great if we can process this PR this week. Thanks.

@DeniseWorthen
Copy link
Collaborator

@jkbk2004 Please note the requested priority for this PR.

@jkbk2004
Copy link
Collaborator

Thanks for reminding! Several priority PRs are going-on. #2044 is on-going. I think we can commit this pr tomorrow.

@zach1221
Copy link
Collaborator

@haiqinli I think we're going to begin regression testing on this PR next. Can you please sync up your branch?

@jkbk2004
Copy link
Collaborator

@haiqinli can you sync up the branches? so we can work on this pr.

@haiqinli
Copy link
Contributor Author

@jkbk2004 @zach1221 Yes, I am starting to sync my branch, and will update when it is done. Thanks.

@zach1221
Copy link
Collaborator

@haiqinli did you intend to close this PR?

@haiqinli
Copy link
Contributor Author

@zach1221 No. I just synced with the authoritative repository.

@haiqinli haiqinli reopened this Dec 21, 2023
@haiqinli
Copy link
Contributor Author

@jkbk2004 @zach1221 The branches have been synced up. Thanks.

@jkbk2004
Copy link
Collaborator

@haiqinli can you re-open NOAA-EMC/fv3atm#728 ?

@haiqinli
Copy link
Contributor Author

@jkbk2004 Reopened. Thanks.

@zach1221 zach1221 added Baseline Updates Current baselines will be updated. Ready for Commit Queue The PR is ready for the Commit Queue. All checkboxes in PR template have been checked. jenkins-ci Jenkins CI: ORT build/test on docker container labels Dec 21, 2023
@epic-cicd-jenkins
Copy link
Collaborator

Jenkins-ci ORTs failed

@jkbk2004
Copy link
Collaborator

@haiqinli @grantfirl @SamuelTrahanNOAA @Qingfu-Liu Can you check if this pr might have some impact on the call cu_gf_deep_run() for middle GF convection cu_gf_driver.F90 line 769 and 856 ? Sounds like memory issue is triggered there.

@hu5970 FYI: all other tests pass except a few Hercules/gnu rap cases.

@SamuelTrahanNOAA
Copy link
Collaborator

I see many big local arrays in cu_gf_deep.F90. One of their dimensions is nchem, which may be zero. If smoke is enabled, nchem=3. What does the Intel and GNU compiler do when a dimension is 0? Do they allocate the array with a dimension of 1? Or is it not allocated at all?

@haiqinli
Copy link
Contributor Author

Actually I also sent the nchem as input to the MYNN PBL, and defined some local arrays in module_bl_mynn.F90.

@haiqinli
Copy link
Contributor Author

In the develop branch, I used the following,
1448 integer :: nchem = 3 !< number of prognostic chemical species (vertically mixied)
1449 integer :: ndvel = 3 !< number of prognostic chemical species (which are deposited, usually =nchem)

In this PR, I changed it as,
1458 integer :: nchem !< number of prognostic chemical species (vertically mixied)
1459 integer :: ndvel !< number of prognostic chemical species (which are deposited, usually =nchem)

@SamuelTrahanNOAA Maybe we should change it back?

@SamuelTrahanNOAA
Copy link
Collaborator

If you're passing in the correct value for nchem, it should be better than 3. When smoke/dust is disabled, nchem=0, and the model should use less memory.

You could try hard-coding it to 3 and see what happens.

@haiqinli
Copy link
Contributor Author

@jkbk2004 Would you like to try hard-coding nchem=3 in ccpp/data/GFS_typedefs.F90 for Hercules/gnu? Or should I update the PR with hard-coding nchem=3? Thanks.

@SamuelTrahanNOAA
Copy link
Collaborator

@jkbk2004 - Can you tell us how to reproduce the problem?

@jkbk2004
Copy link
Collaborator

@haiqinli try to update on physics feature branch. I will try to manually update on hercules/gnu. @SamuelTrahanNOAA the issue only pops up on hercules/gnu. If you don't have access to hercules, I think I will follow up to check on hercules.

@jkbk2004
Copy link
Collaborator

@jkbk2004 Would you like to try hard-coding nchem=3 in ccpp/data/GFS_typedefs.F90 for Hercules/gnu? Or should I update the PR with hard-coding nchem=3? Thanks.

@haiqinli I tried to set in GFS_typedefs.F90

    integer              :: nchem = 3           !< number of prognostic chemical species (vertically mixied)
    integer              :: ndvel = 3          !< number of prognostic chemical species (which are deposited, usually =nchem)

But I see still same issue.

@haiqinli
Copy link
Contributor Author

hercules/gnu.

@jkbk2004 Thank you very much for trying this. I am also checking the code, and will update if I can find something.

@DusanJovic-NOAA
Copy link
Collaborator

Regardless of the value of nchem and ndvel (0 or 3) the arrays like chem3d are never allocated if Model%rrfs_sd is false, which probably is for these rap tests. In that case status of chem3d pointer in disassociated. Disassociated pointer should not be passed to dummy array (which is not pointer itself) with explicit shape, like chem3d is in cu_gf_deep_run.

@SamuelTrahanNOAA
Copy link
Collaborator

Regardless of the value of nchem and ndvel (0 or 3) the arrays like chem3d are never allocated if Model%rrfs_sd is false, which probably is for these rap tests. In that case status of chem3d pointer in disassociated. Disassociated pointer should not be passed to dummy array (which is not pointer itself) with explicit shape, like chem3d is in cu_gf_deep_run.

@haiqinli - I believe @DusanJovic-NOAA's suggestion is to change the dimensions to :

@haiqinli
Copy link
Contributor Author

Regardless of the value of nchem and ndvel (0 or 3) the arrays like chem3d are never allocated if Model%rrfs_sd is false, which probably is for these rap tests. In that case status of chem3d pointer in disassociated. Disassociated pointer should not be passed to dummy array (which is not pointer itself) with explicit shape, like chem3d is in cu_gf_deep_run.

@haiqinli - I believe @DusanJovic-NOAA's suggestion is to change the dimensions to :

@SamuelTrahanNOAA @DeniseWorthen Thank you very much for your suggestions. I will update the feather branch with dimension to ":".

@jkbk2004
Copy link
Collaborator

@haiqinli keep me posted. I will check on hercules.

@haiqinli
Copy link
Contributor Author

@jkbk2004 I have updated the feature brach of ccpp-physics and fv3atm. Thanks.

@DeniseWorthen
Copy link
Collaborator

@haiqinli Did you commit your changes? You say you've updated your branch, but I don't see any update here.

@haiqinli
Copy link
Contributor Author

@DeniseWorthen I am sorry that only the fv3atm and ccpp-physics were updated. The feature branch of ufs-weather-model is just updated. Thanks!

@jkbk2004
Copy link
Collaborator

rap_control_gnu is running ok with the fix on hercules. I will finish remaining test on hercules and push test log. @zach1221 FYI

@jkbk2004
Copy link
Collaborator

FYI: sanity check shows no impact of last commit on existing tests on other machines. So all tests are done. We can start merging process.

@haiqinli
Copy link
Contributor Author

@jkbk2004 Thank you very much for your updates! I am available to work with you to update the submodules.

@jkbk2004 jkbk2004 merged commit 173146c into ufs-community:develop Dec 28, 2023
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. jenkins-ci Jenkins CI: ORT build/test on docker container Ready for Commit Queue The PR is ready for the Commit Queue. All checkboxes in PR template have been checked.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants