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

Update CICE and use EMC/CICE:develop branch from CICE-Consortium:main #2430

Merged

Conversation

NickSzapiro-NOAA
Copy link
Collaborator

@NickSzapiro-NOAA NickSzapiro-NOAA commented Sep 7, 2024

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 Intel+GNU RT suite (compared to current baselines) on either Hera/Derecho/Hercules
  • Commit 'test_changes.list' from previous step

Description:

This PR changes the EMC/CICE branch used in UFS. While EMC/CICE:emc/develop branch has effectively same code and answers as authoritative CICE-Consortium:main branch, development history over years has introduced git commit differences. Merging code histories as well facilitates future two-way code management. For this, EMC commits ahead of Consortium have been merged to Consortium. This also syncs recent commits at Consortium, like an improved physics option for congelation ice formation.

Issuing a new EMC/CICE:develop branch preserves rather than rewrites history.

Commit Message:

* UFSWM - Use EMC/CICE:develop branch from CICE-Consortium:main
  * CICE - Use EMC/CICE:develop branch from CICE-Consortium:main

Priority:

  • Normal

Git Tracking

UFSWM:

Sub component Pull Requests:

  • None

UFSWM Blocking Dependencies:

  • None

Changes

Regression Test Changes (Please commit test_changes.list):

  • No Baseline Changes.

Input data Changes:

  • None.

Library Changes/Upgrades:

  • No Updates

Testing Log:

  • RDHPCS
    • Hera
    • Orion
    • Hercules
    • Jet
    • Gaea
    • Derecho
  • WCOSS2
    • Dogwood/Cactus
    • Acorn
  • CI
  • opnReqTest (complete task if unnecessary)

@NickSzapiro-NOAA
Copy link
Collaborator Author

fyi @NeilBarton-NOAA @rgrumbine @DeniseWorthen , use of new congel_freeze = 'one-step' physics namelist option may be recommended (CICE-Consortium/Icepack#494 ), if anyone wants to do physics runs. It is not default and must be switched on

@NickSzapiro-NOAA NickSzapiro-NOAA changed the title Use EMC/CICE:develop branch from CICE-Consortium:main Update CICE and use EMC/CICE:develop branch from CICE-Consortium:main Sep 8, 2024
.gitmodules Outdated Show resolved Hide resolved
@jkbk2004
Copy link
Collaborator

jkbk2004 commented Sep 17, 2024

I will see if we can apply a fix for fire component submodule pointer to NOAA-EMC/fire_behavior@05cad17 @zach1221 @FernandoAndrade-NOAA @BrianCurtis-NOAA FYI

@jkbk2004
Copy link
Collaborator

NOAA-EMC/fire_behavior@05cad17 reproduces b2b: /scratch2/NCEPDEV/marine/Jong.Kim/UFS-RT/rt-2430/tests/RegressionTests_hera.log. @NickSzapiro-NOAA can you update .gitmodules?

[submodule "fire_behavior"]
  path = fire_behavior
  url = https://github.com/NOAA-EMC/fire_behavior
  branch = emc/develop

@jkbk2004 jkbk2004 added the Ready for Commit Queue The PR is ready for the Commit Queue. All checkboxes in PR template have been checked. label Sep 18, 2024
@zach1221 zach1221 added the jenkins-ort run ORT testing label Sep 18, 2024
@DeniseWorthen
Copy link
Collaborator

@NickSzapiro-NOAA You are updating CICE, but I see no associated CICE PR.

@jkbk2004 You marked the PR as ready. The subcomponent PR (I think there should be one) is not reviewed.

This PR is not ready for the commit Q.

@jkbk2004
Copy link
Collaborator

@NickSzapiro-NOAA can you make sure this pr is to bring up https://github.com/NOAA-EMC/CICE/tree/develop? The develop branch is fully ready, right?

@DeniseWorthen
Copy link
Collaborator

@jkbk2004 This PR will switch to a new develop branch, which includes recent updates from the Consortium. Those updates need a PR.

@NickSzapiro-NOAA
Copy link
Collaborator Author

@DeniseWorthen I don't know how to make an EMC/CICE PR for this. Maybe I made a mistake by creating a https://github.com/NOAA-EMC/CICE/tree/develop branch already during review. I can make an isssue with what is updated but no code should change (via PR)

@DeniseWorthen
Copy link
Collaborator

@NickSzapiro-NOAA Despite the history not aligning, our actual code was always up-to-date w/ Consortium when we made a PR to update the top of our fork. You are doing two things---one, you're renaming the top of our fork, but you're also updating to the the latest in Consortium. You mention that this includes, for example, 'improved physics for congelation ice'. We need updates like that documented in our fork, so we know what changed between the top of the old emc/develop and the top of the new develop.

@NickSzapiro-NOAA
Copy link
Collaborator Author

NickSzapiro-NOAA commented Sep 18, 2024

@DeniseWorthen I've now listed the updates in an updated EMC/CICE issue (NOAA-EMC/CICE#82). Are you asking to delete the current EMC/CICE develop branch to create it via PR? My working idea is that creating a subcomponent branch/tag/release/... can be done at any point but using new branch in UFS requires testing and approval.

@BrianCurtis-NOAA
Copy link
Collaborator

I'm not sure how you wish to handle this, but the current setup passes all RT's on Acorn. If you update the hash to the new develop branch of NOAA-EMC/CICE then that would be the only difference, assuming the switch from emc/develop to develop doesn't change anything in the code.

@NickSzapiro-NOAA
Copy link
Collaborator Author

Thanks @BrianCurtis-NOAA. There are hash and code changes between current emc/develop and develop branches of EMC/CICE

@DeniseWorthen
Copy link
Collaborator

@NickSzapiro-NOAA Thanks for creating the issue. Since the new develop branch is already in place, I guess we can continue from here. I think it might have been better to do it in two steps, but that is my fault for not paying more attention.

@BrianCurtis-NOAA
Copy link
Collaborator

@NickSzapiro-NOAA what are we still waiting on for this PR to start?

@NickSzapiro-NOAA
Copy link
Collaborator Author

Just for testing to complete. The EMC/CICE branch already exists and we use it via .gitmodules as is

@zach1221 zach1221 removed the jenkins-ort run ORT testing label Sep 19, 2024
@zach1221
Copy link
Collaborator

Testing is done. Please give final reviews.

@zach1221 zach1221 merged commit 38a29a6 into ufs-community:develop Sep 19, 2024
3 checks passed
@NickSzapiro-NOAA NickSzapiro-NOAA deleted the branch_CICE-Consortium branch September 20, 2024 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
No Baseline Change No Baseline Change Ready for Commit Queue The PR is ready for the Commit Queue. All checkboxes in PR template have been checked.
Projects
Development

Successfully merging this pull request may close these issues.

8 participants