Skip to content

port for gaeac6#1043

Merged
apcraig merged 12 commits into
CICE-Consortium:mainfrom
rgrumbine:dev/gaea
Aug 8, 2025
Merged

port for gaeac6#1043
apcraig merged 12 commits into
CICE-Consortium:mainfrom
rgrumbine:dev/gaea

Conversation

@rgrumbine
Copy link
Copy Markdown
Contributor

For detailed information about submitting Pull Requests (PRs) to the CICE-Consortium,
please refer to: https://github.com/CICE-Consortium/About-Us/wiki/Resource-Index#information-for-developers

PR checklist

  • Short (1 sentence) summary of your PR:
    Port to geac6
  • Developer(s):
    Robert Grumbine
  • Suggest PR reviewers from list in the column to the right.
  • Please copy the PR test results link or provide a summary of testing completed below.
    quick_suite:
    16 measured results of 16 total results
    16 of 16 tests PASSED
    0 of 16 tests PENDING
    0 of 16 tests MISSING data
    0 of 16 tests FAILED
  • How much do the PR code changes differ from the unmodified code?
    • [X ] bit for bit
    • different at roundoff level
    • more substantial
  • Does this PR create or have dependencies on Icepack or any other models?
    • Yes
    • [X ] No
  • Does this PR update the Icepack submodule? If so, the Icepack submodule must point to a hash on Icepack's main branch.
    • Yes
    • [X ] No
  • Does this PR add any new test cases?
    • Yes
    • [ X] No
  • Is the documentation being updated? ("Documentation" includes information on the wiki or in the .rst files from doc/source/, which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/. A test build of the technical docs will be performed as part of the PR testing.)
    • Yes
    • [X ] No, does the documentation need to be updated at a later time?
      • Yes
      • [ X] No
  • Please document the changes in detail, including why the changes are made. This will become part of the PR commit log.
    Port to gaeac6

Copy link
Copy Markdown
Contributor

@apcraig apcraig left a comment

Choose a reason for hiding this comment

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

Please remove the generic gaea_intel implementation. I thought that was going to be part of this PR.

Please consider updating the gaeac5_intel env variables to specify versions.

Please update the MACHINFO and ENVINFO settings in the env files for gaeac5 and gaeac6. You can find MACHINFO here, https://docs.rdhpcs.noaa.gov/systems/gaea_user_guide.html. For the ENVINFO, I normally load the modules in the env file in a login window just like they're specified. Then I'll do module load to check versions. Then I'll go deeper and do ifort/icc/ifx/icx --version and report that. If you are specifying particular versions of modules, those versions should be static and it's helpful to detail that info in the ENVINFO variable so we can track more easily exactly what each machine is using.

Do you want me to create an inputdata space on gaea? I'm not sure how the scratch space is setup though, I don't seem to have permission anywhere. Is there a space on scratch I should use that is not scrubbed?

@apcraig
Copy link
Copy Markdown
Contributor

apcraig commented Jul 30, 2025

I have not been able to run on gaeac6 yet, but have reviewed the port, built on c5 and c6, and run on c5. Several comments

  • The account should not be hardwired into the batch script. You can set a default in the env file, but a user is unable to set that via command line if it's hardwired.
  • Is the -qos option not working on c6? It was commented out. Recommend changing that.
  • -xHost is set in the c and fortran compilation on c5 and c6. I think this should be -march=core-avx2.
  • c5 does not compile with the current updated env modules with debug. You need to update the debug FFLAGS to fpe1 and remove -check uninit in c5.
  • There is some debug output in the env gaea files that probably should be commented out (module list, env calls)
  • Please check whether the c6 default account should be sfs-emc or sfs-cpu. You had sfs-emc hardwired.

I have created a git diff text file to show how I fixed those things,

gaea:/ncrc/home1/Anthony.Craig/rg.dev.gaea.diffs01

if you want to use that as a reference. I'm also happy to PR these changes to the dev/gaea branch if you agree to these changes and prefer I use that method. Also, please test run quick suites on both c5 and c6 to make sure things are working.

I hope to have access to c6 soon-ish to verify the c6 port. But if that takes time, we can merge once we are both comfortable that things are working well.

@apcraig apcraig added the Porting label Aug 5, 2025
@apcraig
Copy link
Copy Markdown
Contributor

apcraig commented Aug 6, 2025

I'm reviewing and testing again. There are several changes that still should be made based on my prior suggestions. I will create a PR to your branch.

@apcraig
Copy link
Copy Markdown
Contributor

apcraig commented Aug 6, 2025

I have created a PR to the rgrumbine:dev/gaea branch, rgrumbine#1

@apcraig
Copy link
Copy Markdown
Contributor

apcraig commented Aug 7, 2025

I ran a bit of a larger test suite on c5 and the inputdata is a little out of date, many test cases do not run due to missing data. We need to update the inputdata.

@rgrumbine, should we swap the inputdata area to a directory that I own and can keep up to date? I'm also happy to move the missing data onto gaea and then let you copy it into your space, that should work too.

@apcraig
Copy link
Copy Markdown
Contributor

apcraig commented Aug 7, 2025

I moved all the latest inputdata to gaea and updated the inputdata env variable to point to my directories in rgrumbine#1

@apcraig
Copy link
Copy Markdown
Contributor

apcraig commented Aug 7, 2025

With the updated inputdata and other changes in rgrumbine#1, I'm getting consistent results testing quick_suite, base_suite, unittest_suite,

713 measured results of 713 total results
713 of 713 tests PASSED
0 of 713 tests PENDING
0 of 713 tests MISSING data
0 of 713 tests FAILED

If rgrumbine#1 can be reviewed and merged into this PR, then I think this PR could be ready to go.

@apcraig
Copy link
Copy Markdown
Contributor

apcraig commented Aug 8, 2025

This looks good to me. @rgrumbine, are you comfortable with the current implementation, have you confirmed it works for you? Are you OK using --acct and --queue on the cice.setup command line to setup cases on c6 for now?

@rgrumbine
Copy link
Copy Markdown
Contributor Author

It all looks and works fine for me. Thank you

@apcraig apcraig merged commit 997835d into CICE-Consortium:main Aug 8, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants