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

MPAS: Add instance string to output filenames #5123

Merged
merged 3 commits into from
Dec 12, 2022
Merged

MPAS: Add instance string to output filenames #5123

merged 3 commits into from
Dec 12, 2022

Conversation

mkstratos
Copy link
Contributor

@mkstratos mkstratos commented Aug 10, 2022 •
edited by jonbob
Loading

To solve issue #5122:

  • Create a stream file for multiple instances of MPAS Ocean and Sea Ice
  • Adds the instance string to streams.ocean_* and streams.seaice_* output and restart filenames

Fixes #5122

[BFB]

@mkstratos mkstratos added mpas-ocean mpas-seaice bug fix PR Testing Anything related to unit/system tests labels Aug 10, 2022
@mkstratos mkstratos requested a review from xylar August 10, 2022 17:37
@xylar xylar requested review from jonbob and akturner and removed request for xylar August 16, 2022 14:41
@mkstratos mkstratos mentioned this pull request Aug 18, 2022
Open
@rljacob rljacob mentioned this pull request Aug 18, 2022
Closed
@rljacob
Copy link
Member

rljacob commented Aug 25, 2022

@akturner please review

@rljacob rljacob removed the request for review from jonbob August 25, 2022 17:31
@rljacob
Copy link
Member

rljacob commented Sep 15, 2022

@akturner please review.

1 similar comment
@rljacob
Copy link
Member

rljacob commented Oct 6, 2022

@akturner please review.

@mark-petersen mark-petersen self-requested a review October 11, 2022 19:52
@mark-petersen
Copy link
Contributor

Compiled stand-alone MPAS-Ocean with gnu debug and intel debug. Passes nightly suite for both. This is not surprising because ocn_comp_mct.F is the only file altered in this PR that affects stand-alone. I tested

SMS_D_Ln9.T62_oQU120_ais20.MPAS_LISIO_TEST.cori-haswell_gnu

and it failed the run step, but the error was not clear. Since cori is nearing its end, I'll test on other machines.

@rljacob
Copy link
Member

rljacob commented Nov 3, 2022

telecon notes: Needs another review.

mark-petersen
mark-petersen approved these changes Nov 7, 2022
View reviewed changes
Copy link
Contributor

@mark-petersen mark-petersen left a comment

Choose a reason for hiding this comment

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

This passes

./create_test SMS_D.T62_oQU120_ais20.MPAS_LISIO_TEST.anvil_gnu -q acme-small --walltime 30:00

and the changes all look reasonable. Thanks.

@rljacob rljacob added this to the v2.1rc milestone Dec 1, 2022
jonbob added a commit that referenced this pull request Dec 6, 2022
@jonbob
Merge branch 'mkstratos/mpas/mpas-multi-instance' into next (PR #5123)
41efb69 
Add instance string to MPAS output filenames

Creates a stream file for multiple instances of MPAS Ocean and Sea Ice.
Adds the instance string to streams.ocean_* and streams.seaice_* output
and restart filenames

Fixes #5122

[BFB]
@jonbob
Copy link
Contributor

jonbob commented Dec 6, 2022

passes:

  • ERS.ne11_oQU240.WCYCL1850NS.chrysalis_intel
  • SMS_D_Ld3.T62_oQU120.CMPASO-IAF.chrysalis_intel
  • SMS_D_Ld1.ne30pg2_EC30to60E2r2.WCYCL1850.chrysalis_intel.allactive-wcprod

merged to next

@ndkeen ndkeen mentioned this pull request Dec 7, 2022
Closed
jonbob added a commit that referenced this pull request Dec 8, 2022
@jonbob
Re-merge branch 'mkstratos/mpas/mpas-multi-instance' into next (PR #5123
aee1827 
)

Re-merge to fix issue with branch/hybrid tests
@jonbob
Copy link
Contributor

jonbob commented Dec 8, 2022

re-merged to next with fix for branch and hybrid tests. Now passes:

  • SMS_Ld1.ne30pg2_EC30to60E2r2.WCYCLSSP370.chrysalis_intel.allactive-wcprodssp
  • ERS.ne30pg2_EC30to60E2r2.WCYCLSSP370.chrysalis_intel.allactive-wcprodssp

@jonbob
Copy link
Contributor

jonbob commented Dec 12, 2022

The current NCK test failures are related to the mpas files now having instance strings in their names, as they should, and not having baselines for those filenames.

@jonbob jonbob merged commit 7c078d7 into master Dec 12, 2022
@jonbob jonbob deleted the mkstratos/mpas/mpas-multi-instance branch December 12, 2022 17:26
@jonbob
Copy link
Contributor

jonbob commented Dec 12, 2022

merged to master and NCK test DIFFs blessed

xylar added a commit to xylar/compass that referenced this pull request Dec 15, 2022
@xylar
Update E3SM-Project submodule
21f8a56 
This merge updates the E3SM-Project submodule from [569ed6b730](https://github.com/E3SM-Project/E3SM/tree/569ed6b730) to [0273cfad9d](https://github.com/E3SM-Project/E3SM/tree/0273cfad9d).

This update includes the following MPAS-Ocean and MPAS-Frameworks PRs (check mark indicates bit-for-bit with previous PR in the list):
- [ ]  (ocn) E3SM-Project/E3SM#5306
- [ ]  (fwk) E3SM-Project/E3SM#5303
- [ ]  (ocn) E3SM-Project/E3SM#5325
- [ ]  (fwk) E3SM-Project/E3SM#5337
- [ ]  (fwk) E3SM-Project/E3SM#5123
- [ ]  (fwk) E3SM-Project/E3SM#5281
- [ ]  (ocn) E3SM-Project/E3SM#5356
@xylar xylar mentioned this pull request Dec 15, 2022
Merged
9 tasks
@ndkeen
Copy link
Contributor

ndkeen commented Dec 16, 2022

Do we need to now bless the NCK test on all machines?

@jonbob
Copy link
Contributor

jonbob commented Dec 16, 2022

@ndkeen -- I did bless them, but apparently the baselines need to be regenerated instead of simply blessed. @jgfouca and I worked on this yesterday and he came up with some commands to add to the bless in order to do so. So I made new requests for that yesterday, at least for chrysalis. The other machine reporting this failure is perlmutter and it's still down.

@jonbob
Copy link
Contributor

jonbob commented Dec 16, 2022

@ndkeen - on today's cdash for chrysalis-next, it now appears as a NML DIFF instead of a regular DIFF. I submitted a request to bless the NML DIFF as well

@ndkeen
Copy link
Contributor

ndkeen commented Dec 16, 2022

OK great -- you're on it. I just didn't want to lose the fact that I tracked down an issue to this PR and we had to re-investigate later.

Note, I learned you can add commenter:@me into github search to show issues/PR's that you commented on (which doesnt count as a mention).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mark-petersen mark-petersen mark-petersen approved these changes

@akturner akturner akturner approved these changes

Assignees

@jonbob jonbob

Labels
bug fix PR mpas-ocean mpas-seaice Testing Anything related to unit/system tests
Projects
None yet
Milestone
v2.1rc
Development

Successfully merging this pull request may close these issues.

Multi-instance runs stall at last timestep
6 participants
@mkstratos @rljacob @mark-petersen @jonbob @ndkeen @akturner