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

Adding IcoswISC30E3r5 ice initial condition with DIB #6221

Merged
merged 3 commits into from
Feb 21, 2024

Conversation

darincomeau
Copy link
Member

@darincomeau darincomeau commented Feb 8, 2024

This adds an initial condition for MPAS-Seaice for cases where the IcoswISC30E3r5 mesh is paired with a CRYO* compset, which utilize data iceberg (DIB) forcing. The new initial condition is from a restart from a one month GMPAS-JRA1p5-DIB-PISMF case.

The current sea ice IC for this mesh in B-cases ('spun-up' ICs) was not run with DIB forcing, and as such MPAS-Seaice cannot use it as a restart for a case with DIB.

This also adds CRYO1850 and CRYO1850-DISMF tests with this mesh to the e3sm_integration test suite, which prior to this would not pass.

[BFB] Only impacts configurations with IcoswISC30E3r5 and a CRYO* compset, which prior to this would seg fault in ice during initialization.

Credit to @xylar for rooting out the problem.

@darincomeau
Copy link
Member Author

Tested

SMS.ne30pg2_IcoswISC30E3r5.WCYCL1850.chrysalis_intel, confirmed existing sea ice IC is still used.

SMS.ne30pg2_IcoswISC30E3r5.CRYO1850.chrysalis_intel,
SMS.ne30pg2_IcoswISC30E3r5.CRYO1850-DISMF.chrysalis_intel
confirmed new sea ice IC is used.

@xylar
Copy link
Contributor

xylar commented Feb 8, 2024

@darincomeau, would you be willing to cherry-pick the commit from E3SM-Ocean-Discussion#77 onto this branch? I think it would be good to add a test that would have caught this issue so we make sure this configuration stays functional.

@darincomeau
Copy link
Member Author

@xylar I agree on adding the test, thanks for adding that to Ocean-Discussion, I missed that. I can add it here, but do you think it should be it's own PR?

@xylar
Copy link
Contributor

xylar commented Feb 8, 2024

@darincomeau, I think we should add those tests here because a big part of the motivation in adding them is to show that the configurations work that this PR is meant to fix. I can split them into a separate PR if there are objections to including them here.

Copy link
Contributor

@xylar xylar left a comment

Choose a reason for hiding this comment

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

I ran both of the following tests successfully, whereas they failed without the changes on this branch:

SMS_D_Ld1.ne30pg2_r05_IcoswISC30E3r5.CRYO1850.chrysalis_intel
SMS_D_Ld1.ne30pg2_r05_IcoswISC30E3r5.CRYO1850-DISMF.chrysalis_intel

Ideally, I'd like to have them added to this branch but I won't let that hold up the merge.

@darincomeau
Copy link
Member Author

Ok I understand. I've cherry-picked that test commit over here and updated the description above.

@jonbob let us know if there's concern on the content here.

@darincomeau darincomeau added the Testing Anything related to unit/system tests label Feb 9, 2024
@darincomeau darincomeau requested review from jonbob and removed request for proteanplanet February 13, 2024 15:40
@jonbob
Copy link
Contributor

jonbob commented Feb 13, 2024

@rljacob -- this PR adds two new tests to e3sm_integration. Do you want to move one to e3sm_prod, since they really represent a cryo production configuration? I can run tests using the testing pelayout and document how long they take to run

@rljacob
Copy link
Member

rljacob commented Feb 13, 2024

Sure move one of the cryo cases to production.

cime_config/tests.py Outdated Show resolved Hide resolved
@darincomeau
Copy link
Member Author

Also, I'm not sure where is a good place to document this, but cases without DIB can use the sea ice IC with DIB, so for future meshes we can just make one IC with DIB to avoid the extra file.

jonbob added a commit that referenced this pull request Feb 20, 2024
Adding IcoswISC30E3r5 ice initial condition with DIB

This adds an initial condition for MPAS-Seaice for cases where the
IcoswISC30E3r5 mesh is paired with a CRYO* compset, which utilize data
iceberg (DIB) forcing. The new initial condition is from a restart from
a one month GMPAS-JRA1p5-DIB-PISMF case.

The current sea ice IC for this mesh in B-cases ('spun-up' ICs) was not
run with DIB forcing, and as such MPAS-Seaice cannot use it as a restart
for a case with DIB.

This also adds CRYO1850 and CRYO1850-DISMF tests with this mesh to the
e3sm_integration test suite, which prior to this would not pass.

[BFB] Only impacts configurations with IcoswISC30E3r5 and a CRYO*
      compset, which prior to this would seg fault in ice during
      initialization
Copy link
Contributor

@jonbob jonbob left a comment

Choose a reason for hiding this comment

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

approved based on visual inspection and testing

@jonbob
Copy link
Contributor

jonbob commented Feb 20, 2024

Passes:

  • SMS_Ld1.ne30pg2_r05_IcoswISC30E3r5.WCYCL1850.chrysalis_intel.allactive-wcprod_1850_r05

Also verified that both new tests:

  • SMS_D_Ld1.ne30pg2_r05_IcoswISC30E3r5.CRYO1850-DISMF
  • SMS_D_Ld1.ne30pg2_r05_IcoswISC30E3r5.CRYO1850
    pass using the chrysalis testing layout and that the new file is available on the inputdata repo

merged to next

@jonbob jonbob merged commit 80bd7a2 into E3SM-Project:master Feb 21, 2024
3 checks passed
@jonbob
Copy link
Contributor

jonbob commented Feb 21, 2024

merged to master and new baselines requested for compy_prod, chrysalis_prod, and chrysalis_integration. Will need to do the same for pm-cpu_integration when it reports next, as well as anvil_prod which is failing all tests with the new Icos mesh. I'm currently testing to see if those fails are related to the size of the test pelayout on anvil

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BFB PR leaves answers BFB Coupled Model mpas-seaice Testing Anything related to unit/system tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants