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

Enable DA cycling on gaea #3032

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Enable DA cycling on gaea #3032

wants to merge 2 commits into from

Conversation

jswhit2
Copy link
Contributor

@jswhit2 jswhit2 commented Oct 23, 2024

Description

currently DA cycling on gaea is disabled. These changes remove that restriction.

Type of change

New feature (adds functionality)

Change characteristics

  • Is this a breaking change (a change in existing functionality)? /NO
  • Does this change require a documentation update? NO
  • Does this change require an update to any of the following submodules? NO

How has this been tested?

ran a cycled test with coupled 3dvar (0.5 degree atmosphere and 0.25 degree ocean).

Checklist

  • Any dependent changes have been merged and published
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have documented my code, including function, input, and output descriptions
  • My changes generate no new warnings
  • New and existing tests pass with my changes
  • This change is covered by an existing CI test or a new one has been added
  • Any new scripts have been added to the .github/CODEOWNERS file with owners
  • I have made corresponding changes to the system documentation if necessary

Copy link
Contributor

@CoryMartin-NOAA CoryMartin-NOAA left a comment

Choose a reason for hiding this comment

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

Did not test but looks reasonable. This is for C5, do we know how easy it would be to work on C6?

@jswhit2
Copy link
Contributor Author

jswhit2 commented Oct 23, 2024

c6 is basically the same - there are only subtle differences in some of the system libraries. This means that some of the executables compiled on c5 won't run on c6 (the model does, but GSI doesn't). So, the biggest obstacle to running on c6 will be to update all the build systems to have separate c5/c6 modulefiles.

@CoryMartin-NOAA
Copy link
Contributor

Thanks @jswhit2 for the explanation

@WalterKolczynski-NOAA WalterKolczynski-NOAA changed the title changes to enable DA cycling on gaea Enable DA cycling on gaea Oct 30, 2024
Copy link
Contributor

@WalterKolczynski-NOAA WalterKolczynski-NOAA left a comment

Choose a reason for hiding this comment

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

It appears this PR is specifically to enable cycling for the JEDI system? The atm-only GSI-based test at least appears to already be enabled on Gaea.

If this PR enables cycling with the JEDI system, those tests (in ci/cases/pr) should probably be tested and enabled.

env/GAEA.env Outdated
;;
*)
# Some other job not yet defined here
echo "WARNING: The job step ${step} does not specify Hercules-specific resources"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
echo "WARNING: The job step ${step} does not specify Hercules-specific resources"
echo "WARNING: The job step ${step} does not specify Gaea-specific resources"

Copy link
Contributor

Choose a reason for hiding this comment

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

@WalterKolczynski-NOAA I have been running the coupled atm/ocean 3dvar, so I wasn't aware it already worked for atm only DA. When running coupled DA without these mods, load_ufsda_modules.sh quits with the message "WARNING: UFSDA NOT SUPPORTED ON THIS PLATFORM".

@WalterKolczynski-NOAA
Copy link
Contributor

Should coordinate with @AnilKumar-NOAA as well, who has been working on the Gaea port.

@AnilKumar-NOAA
Copy link
Contributor

Yes, I'll port and test on gaea but only after all recent OS upgrades on gaea PR's are merged. We are waiting on that.

@DavidBurrows-NCO
Copy link
Contributor

Hi @jswhit2 @WalterKolczynski-NOAA @CoryMartin-NOAA @AnilKumar-NOAA...Walter is correct, the C96_atm3DVar test case PR2885 was merged for Gaea-C5. However, with the recent C5 OS upgrade, all submodules within the global-workflow fail to build. After talking with Rahul, we decided to submit PRs to build submodules on both C5 and C6 and to follow the filename updates of the ufs-wx-model PR2448, e.g., having a gaeac5 and gaeac6 module files, having detect machine scripts return gaeac5 or gaeac6, etc. I have submitted PRs for Gaea-C5 and C6 builds for ufs-utils, gfs-utils, upp, gsi, gsi-monitor, and gsi-utils. I could set up a GDAS C5 and C6 build PR if you want/need. I'm in training full time through Thursday just to note.

@jswhit2
Copy link
Contributor Author

jswhit2 commented Nov 1, 2024

I could set up a GDAS C5 and C6 build PR if you want/need. I'm in training full time through Thursday just to note.

yes please, that would be great. I'd be happy to do the testing needed to make sure things work on c5/c6 once all the builds work.

@DavidBurrows-NCO
Copy link
Contributor

I could set up a GDAS C5 and C6 build PR if you want/need. I'm in training full time through Thursday just to note.

yes please, that would be great. I'd be happy to do the testing needed to make sure things work on c5/c6 once all the builds work.

@jswhit2 Sounds good. I submitted an issue to build GDASApp on Gaea-C5 and C6 here. I will keep you updated on build progress. Is test data already set up on both systems?

@DavidBurrows-NCO
Copy link
Contributor

@jswhit As @CoryMartin-NOAA said, C6 build was already set up. I made adjustments to C5 module files and machine ID to work with how the global-workflow will be configured, i.e., using gaeac5 and gaeac6 throughout global-workflow and submodules. I have a branch here that builds on both C5 and C6 with those C5 updates. @jswhit Let me know if you want to run tests on this set up, or since the changes are minimal, I may just submit this PR for building. Thanks

@DavidBurrows-NCO
Copy link
Contributor

@jswhit Since your PR is for the global-workflow side, I submitted the GDAS PR1361 .

CoryMartin-NOAA pushed a commit to NOAA-EMC/GDASApp that referenced this pull request Nov 6, 2024
…al-workflow (#1361)

After the recent Gaea-C5 OS upgrade, GDASApp fails to build.
This issue corrects Gaea-C5 build and updates the build scripts to
conform to ufs-wx-model (following ufs-wx-model
ufs-community/ufs-weather-model#2448) and
eventual global-workflow updates.

Refs NOAA-EMC/global-workflow 3011
NOAA-EMC/global-workflow#3011
Refs NOAA-EMC/global-workflow 3032
NOAA-EMC/global-workflow#3032
Resolves #1360
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.

6 participants