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

FATES API 35 compatibilty #6419

Merged
merged 8 commits into from
Jun 17, 2024
Merged

Conversation

rgknox
Copy link
Contributor

@rgknox rgknox commented May 15, 2024

The FATE API has been updated which includes

  1. control over the dimensionality of FATES history output and
  2. an update to the parameter file format. This set of changes makes
    E3SM compatible with FATES tag sci.1.76.3_api.35.1.0.

[BFB] except for FATES

@rgknox
Copy link
Contributor Author

rgknox commented May 15, 2024

This PR is a work in progress. At the least, an updated FATES parameter file has to be loaded and pointed to as the default, before tests can complete.

@glemieux glemieux changed the title FATES API 35 compatibiltiy [WIP] FATES API 35 compatibiltiy May 15, 2024
@glemieux glemieux self-assigned this May 15, 2024
@rljacob rljacob assigned bishtgautam and unassigned glemieux May 25, 2024
@glemieux
Copy link
Contributor

glemieux commented May 29, 2024

Running fates test list on perlmutter. If the fates tests pass, this will be ready for review.

@glemieux
Copy link
Contributor

All tests ran successfullly except for fates_cold_allvars test and ERS_Ld60.f45_g37.IELMFATES.pm-cpu_intel.elm-fates_cold_pphys.

The former is due to needing to update the history output list for the testmod, which I've fixed with bd00c67. I'm retesting that now.

The later is a little weirder as it hit the wall clock limit, even though the gnu version of this test ran successfully well under the limit. I'm resubmitting this to see if its a glitch of some sort.

Setting fates_hist_dimlevel with a single line (i.e. = 2,2) fails during
ELMBuildNamelist.  Adjust the assignment to accomodate this.

This also removes hist_ndens to avoid issue E3SM-Project#1106
@glemieux glemieux changed the title [WIP] FATES API 35 compatibiltiy FATES API 35 compatibiltiy May 29, 2024
@glemieux glemieux changed the title FATES API 35 compatibiltiy FATES API 35 compatibilty May 29, 2024
@glemieux
Copy link
Contributor

Resubmitting ERS_Ld60.f45_g37.IELMFATES.pm-cpu_intel.elm-fates_cold_pphys resulted in a succesful run with a much faster run time.

The fates_cold_allvars test is passing now as well after including an additional commit (9be6081) to change how the fates_history_dimlevel is being set. For some reason that I could quite figure out, we can't use fates_history_dimlevel = 2,2 as this would cause an ELMBuildNamelist error during setup. Splitting the setting and being explicit about the index that was being set works, however.
perlmutter location for the allvars test: /global/homes/g/glemieux/scratch/e3sm-tests/pr6419-allvarsrerun-splitarray.fates.pm-cpu.gnu.Cbd00c6724c-Ff0185f7c

@bishtgautam this is ready to review.

@rgknox
Copy link
Contributor Author

rgknox commented May 30, 2024

Thanks for applying that fix @glemieux

@bishtgautam bishtgautam self-requested a review May 30, 2024 13:48
Copy link
Contributor

@bishtgautam bishtgautam left a comment

Choose a reason for hiding this comment

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

Looks good and I have one minor comment for you to consider.

components/elm/src/main/elmfates_interfaceMod.F90 Outdated Show resolved Hide resolved
@glemieux
Copy link
Contributor

glemieux commented Jun 4, 2024

FYI @bishtgautam there's a recent fix (NGEET/fates#1208) to a fates-side issue related to this API update that I'd like to point this PR at once it's done testing. ETA for it is sometime later today.

@glemieux
Copy link
Contributor

glemieux commented Jun 5, 2024

FYI @bishtgautam there's a recent fix (NGEET/fates#1208) to a fates-side issue related to this API update that I'd like to point this PR at once it's done testing. ETA for it is sometime later today.

@bishtgautam The testing on the fates side for the new PR is running into snags, so I'm going stick with the current fates tag that this PR is pointing to now.

Regression testing on pm-cpu against master baseline using the e3sm_land_developer list comes back with B4B results on all non-fates test except the 1x1_smallvilleIA tests. I think this is consistent with results looking back on the Cdash tests from when the perlmutter master was created. All fates tests diffs are as expected.

@glemieux
Copy link
Contributor

@bishtgautam I've updated this PR with the latest fates tag having completed testing and integrated NGEET/fates#1208. I've retested this PR and the results are still the same as prior. This is good to integrate.

bishtgautam added a commit that referenced this pull request Jun 14, 2024
The FATE API has been updated which includes
1) control over the dimensionality of FATES history output and
2) an update to the parameter file format. This set of changes makes
   E3SM compatible with FATES tag sci.1.76.3_api.35.1.0.

[BFB] except for FATES
@bishtgautam bishtgautam merged commit f14a3cf into E3SM-Project:master Jun 17, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants