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.

Sign up for GitHub

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

Jump to bottom

Fix -check all runtime errors (for CCPP) using zero-size allocations #2102

Closed
wants to merge 18 commits into from
Closed

Fix -check all runtime errors (for CCPP) using zero-size allocations #2102

wants to merge 18 commits into from

Conversation

grantfirl
Copy link
Collaborator

@grantfirl grantfirl commented Jan 18, 2024 •
edited
Loading

Commit Queue Requirements:

  • Fill out all sections of this template.
  • All sub component pull requests have been reviewed by their code managers.
  • Run the full RT suite (compared to current baselines) on either Hera/Derecho/Hercules AND have committed the log to my PR branch.
  • Add list of all failed regression tests in "Regression Tests" section.

PR Information

Description

This PR at least partially addresses #2023 (at least any runtime errors associated with the CCPP). The strategy is to simply make sure that every variable is allocated, although for variables that had been conditionally-allocated to save memory, they are now being allocated with zero size. This has a ripple effect in other repos where one needs to change from checking whether a variable is allocated to whether the size is > 0.

The change to Intel.cmake was used for testing only and I can remove this if we're not ready to permanently add it.

Commit Message

Add zero-size variable allocations to pass the -check all compiler option for CCPP-related code.

Priority

  • Critical Bugfix (This PR contains a critical bug fix and should be prioritized.)
  • High (This PR contains a feature or fix needed for a time-sensitive project (eg, retrospectives, implementations))
  • Normal

Blocking Dependencies

Git Issues Fixed By This PR

Partially addresses #2023

Changes

Subcomponent (with links)

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 REGRESSION TESTS

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

Testing Log:

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

@grantfirl grantfirl marked this pull request as ready for review January 22, 2024 17:57
binli2337 and others added 6 commits February 1, 2024 16:35
@binli2337
Consider sea surface currents in air-sea flux calculation for ufs atm…
db98161 
…osphere-ocean coupling (ufs-community#2104)

Enable the capability of considering sea surface current's impact on air-sea flux calculation for atmosphere-ocean coupling. A namelist option is added to control whether or not to use this feature (default is off). Fixes ufs-community#1975

Co-authored by: @binli2337, @BinLiu-NOAA, @WeiguoWang-NOAA, @BijuThomas-NOAA, @hyunsookkim-NOAA, @XuLi-NOAA, @MariaAristizabal-NOAA, @JohnSteffen-NOAA
@uturuncoglu
Bring CDEPS inline capability to CMEPS (ufs-community#2028)
8c9339f 
Developed and enabled using CMEPS with inline CDEPS capability for UFS regional coupling.
Two HAFS regional moving nest atm_ocn_wav coupling regression tests were added to test this capability.
Note: This is a collaborative effort among the ESMF team, EMC/EIB, and EMC hurricane project team.
@GeorgeGayno-NOAA
New regression tests for V2 surface coldstart files (ufs-community#2005)
02ecf10 
- Add three new regression tests for version 2 of the surface coldstart file. (Closes ufs-community#1977)
@JiliDong-NOAA
Post update (ufs-community#2113)
b4c59bb 
UPP will be brought to the commit to fix the missing reflectivity bug for NSSL MP cases.
@ulmononian
Updates for Gaea C5 & F2/F5 Transition + Fix out of bound errors in b…
760e002 
…lock_atmos_copy routines in fv3atm ufs-community#2124 (ufs-community#2115)

- Gaea C5 modulefile & DISKNM update: closes issues Gaea F2 to F5 file system migration ufs-community#2101
- Bring in the global-workflow detect_machine.sh to keep consistent between projects. (Closes Bring in detect_machine.sh 
   from global workflow for consistency across the community. ufs-community#2096 )
- Fix out of bound errors in block_atmos_copy routines in fv3atm
@JessicaMeixner-NOAA
Add extra variables to get ice temperature for marine DA (ufs-communi…
56e31ff 
…ty#2118)

Add ice temperature in CICE output
@jkbk2004
Copy link
Collaborator

@grantfirl can you sync up? I want to test on hercules/gnu.

@grantfirl
Copy link
Collaborator Author

@grantfirl can you sync up? I want to test on hercules/gnu.

@jkbk2004 Done

@jkbk2004
Copy link
Collaborator

@grantfirl This PR runs ok on hercules/gnu. Please, go ahead to sync up branch one more time. We can work on this pr. @zach1221 @FernandoAndrade-NOAA Can you pick up from here? @BrianCurtis-NOAA FYI.

@grantfirl grantfirl changed the base branch from develop to production/RRFS.v1 February 15, 2024 19:25
@grantfirl
Copy link
Collaborator Author

@grantfirl This PR runs ok on hercules/gnu. Please, go ahead to sync up branch one more time. We can work on this pr. @zach1221 @FernandoAndrade-NOAA Can you pick up from here? @BrianCurtis-NOAA FYI.

Recall that this needs to go into the production/RRFS.v1 branch that was just created. I just updated the target, and it looks like there are a bunch of unintended changes, likely because the release branch was cut from an older commit whereas my changes started from the top of develop. I think that I may need to do some surgery here, or perhaps just close this PR and start fresh with the release branch and then add my changes.

@zach1221 @FernandoAndrade-NOAA @BrianCurtis-NOAA @MatthewPyle-NOAA

@jkbk2004
Copy link
Collaborator

@grantfirl thanks for the note! @MatthewPyle-NOAA we can capture subcomponent production branch with this pr. @zach1221 @FernandoAndrade-NOAA @BrianCurtis-NOAA we need a bit more time, I think.

@jkbk2004
Copy link
Collaborator

@grantfirl I am ok to close and open a new one.

@grantfirl
Copy link
Collaborator Author

This is replaced by #2158 which points to the RRFS release branch. This will likely NOT need to go to the develop branch since a longer-term fix is forthcoming.

@grantfirl grantfirl closed this Feb 27, 2024
@grantfirl grantfirl mentioned this pull request Feb 27, 2024
Merged
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@grantfirl @jkbk2004 @binli2337 @uturuncoglu @GeorgeGayno-NOAA @JiliDong-NOAA @ulmononian @JessicaMeixner-NOAA