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

Fix restart error on maint-2.0 #6206

Conversation

peterdschwartz
Copy link
Contributor

@peterdschwartz peterdschwartz commented Feb 3, 2024

Add endwb to ELM restart file in maint-2.0.

[BFB]
Fixes #6195

@peterdschwartz peterdschwartz added bug fix PR maint-2.0 Anything concerning the maint-2.0 branch. labels Feb 3, 2024
@peterdschwartz
Copy link
Contributor Author

peterdschwartz commented Feb 3, 2024

Need to check if maint-2.1 will also need PR #5811

@peterdschwartz
Copy link
Contributor Author

I ran SMS_D_Ld1.ne30pg2_EC30to60E2r2.WCYCLSSP370.pm-cpu_intel.allactive-wcprodssp with no issue, so leaving this as is.

@ndkeen
Copy link
Contributor

ndkeen commented Feb 5, 2024

I might suggest adding in the fix in 5811. I think I needed that for the tests in maint-2.1 to pass. I had meant to add it here as well anyway, but the e3sm_production tests did pass.

@peterdschwartz
Copy link
Contributor Author

Went ahead and made that change as well.

@ndkeen
Copy link
Contributor

ndkeen commented Feb 6, 2024

OK, you likely know more about why this is or is not a good idea.

I did try SMS_Ld1.ne30pg2_EC30to60E2r2.WCYCLSSP585.pm-cpu_intel.allactive-wcprodssp
with and without that finidat and it seemed ok?

@rljacob rljacob added the Land label Feb 6, 2024
@rljacob rljacob self-assigned this Feb 6, 2024
@rljacob
Copy link
Member

rljacob commented Feb 6, 2024

@ndkeen can an ERS test pass on maint-2.0 without these fixes?

@ndkeen
Copy link
Contributor

ndkeen commented Feb 6, 2024

No, before these changes ERS_Lm3.ne4_oQU240.F2010 would fail. But it also fails with even old maint-2.0 checkouts -- ie I think the test would have always failed.

@rljacob
Copy link
Member

rljacob commented Feb 6, 2024

Oh that's right, you have to run it for a long time. A standard-length ERS passes. How long does Lm3 take? We should add an ERS test to the prod suite for maint-2.0 to catch this.

@ndkeen
Copy link
Contributor

ndkeen commented Feb 6, 2024

My test that passes after this fix used a job that ran for a total of 8:45 on 1 node of pm-cpu.

@mingxuanwupnnl
Copy link
Contributor

@peterdschwartz I met the same issue while I was doing production runs. I wonder if this bug fix PR is BFB? Thank you.

@peterdschwartz
Copy link
Contributor Author

@peterdschwartz I met the same issue while I was doing production runs. I wonder if this bug fix PR is BFB? Thank you.

It's BFB

@rebassoo
Copy link
Contributor

@ndkeen, will this PR be merged into maint-2.0 soon? What additional reviews/checks are necessary? thanks

@rljacob rljacob merged commit d2ec49b into E3SM-Project:maint-2.0 Feb 13, 2024
2 checks passed
@rljacob
Copy link
Member

rljacob commented Feb 13, 2024

@rebassoo merged.

@rebassoo
Copy link
Contributor

Great, thanks @rljacob!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fix PR Land maint-2.0 Anything concerning the maint-2.0 branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants