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

Update conditions for Southern Ocean ice and river runoff removal #6693

Merged
merged 3 commits into from
Nov 13, 2024

Conversation

cbegeman
Copy link
Contributor

@cbegeman cbegeman commented Oct 16, 2024

This PR modifies the conditions for river and ice runoff removal in E3SM cases. This changes the behavior of E3SM cases where data icebergs (DIB) or ice-shelf melting (ISMF) were active and thus doesn't not affect water cycle simulations.

River runoff

  • Previous behavior: never removed
  • New behavior: removed when atmosphere is not active and ice shelf melt fluxes are on

Ice runoff

  • Previous behavior: removed when DIB is on and atmosphere is active
    • The 'Previous behavior' is a bit more complicated than described with G-cases, since those relied on removed runoff fields in modified data runoff files. With CORE-II and JRA1-4 forcing, we supported being able to remove AIS liquid/solid runoff independently based on whether ISMF/DIB was active; for JRA1p5 we only had the option to remove both AIS runoff fields if both ISMF & DIB were active (no compsets or corresponding modified runoff files were made with one but not the other active).
  • New behavior: removed when DIB is on

[BFB] when DIB and DISMF are inactive (all WC configurations)
[non-BFB] when DIB or DISMF are active

@cbegeman cbegeman added mpas-ocean non-BFB PR makes roundoff changes to answers. BFB PR leaves answers BFB and removed non-BFB PR makes roundoff changes to answers. labels Oct 16, 2024
@cbegeman cbegeman added non-BFB PR makes roundoff changes to answers. and removed BFB PR leaves answers BFB labels Oct 17, 2024
@xylar
Copy link
Contributor

xylar commented Oct 17, 2024

Great, thanks @cbegeman! I'll run a few quick tests with various combinations of G and B cases and with or without DIB and DISMF

@cbegeman
Copy link
Contributor Author

Testing

I have run a comparison between this branch and master with the e3sm_cryo_developers suite on chrys with intel, impi.

The result was NLFAIL ${TEST_NAME} (but otherwise OK) RUN for all tests. In each case, the master namelist included

config_remove_ais_ice_runoff = .false.
config_remove_ais_river_runoff = .false.

whereas for this branch those options are

config_remove_ais_ice_runoff = .true.
config_remove_ais_river_runoff = .true.

@cbegeman
Copy link
Contributor Author

@xylar If you could run a few tests other than the ones in the cryo dev suite or tests with a different machine, compiler combo than chrys, intel, impi that would be great!

@xylar
Copy link
Contributor

xylar commented Oct 17, 2024

Okay, I'll do that.

I don't think I would have expected the cryo developer tests to be BFB. Was that a surprise to you, too?

@cbegeman
Copy link
Contributor Author

I don't think I would have expected the cryo developer tests to be BFB. Was that a surprise to you, too?

Yes, it was. There are both CORE-IAF and JRA-forced runs in the suite and both should have river runoff and JRA should have ice runoff. Do you think these tests could just be too short?

@xylar
Copy link
Contributor

xylar commented Oct 17, 2024

Do you think these tests could just be too short?

I wouldn't have thought so. I think they run for ~5 days, which should be long enough for removed runoff to affect ocean T and S. But maybe not fields the coupler sees? If not, that would be disconcerting!

This may point to another situation (G cases) where the fact that we don't look at MPAS-Ocean or -Seaice history files is a disaster waiting to happen.

@jonbob, do you have thoughts on this?

@jonbob
Copy link
Contributor

jonbob commented Oct 17, 2024

We usually see impacts in the cpl fields, but not always. But I would expect those tests to be non-BFB. Have we run longer comparisons? I don't see any referenced in this PR

@xylar
Copy link
Contributor

xylar commented Oct 17, 2024

No, we haven't. We presumably need to run a longer Icos G case with master and with this to show that we're not seeing unexpected differences. That would be certain to be non-BFB.

@xylar
Copy link
Contributor

xylar commented Oct 17, 2024

I can get that going.

@jonbob
Copy link
Contributor

jonbob commented Oct 17, 2024

I think it would be necessary to document the impact of this PR

@xylar
Copy link
Contributor

xylar commented Oct 17, 2024

@jonbob, would a 10-year G-case be sufficient? I would think so.

@jonbob
Copy link
Contributor

jonbob commented Oct 17, 2024

@xylar -- I agree, a 10-year G-case comparison would be perfect

@cbegeman
Copy link
Contributor Author

@xylar Thanks for doing that test!

@darincomeau
Copy link
Member

Ice runoff
Previous behavior: removed when DIB is on and atmosphere is not active

This should be when DIB is on and atmosphere is active, correct?

The 'Previous behavior' is a bit more complicated than described with G-cases, since those relied on removed runoff fields in modified data runoff files. With CORE-II and JRA1-4 forcing, we supported being able to remove AIS liquid/solid runoff independently based on whether ISMF/DIB was active; for JRA1p5 we only had the option to remove both AIS runoff fields if both ISMF & DIB were active (no compsets or corresponding modified runoff files were made with one but not the other active).

I think we should test that the GMPAS-JRA1p5-DIB-PISMF compset prior to this PR (where removed runoff is handled by pointing to *no_rofi_no_rofl runoff files) is BFB with this PR (where runoff is removed in MPAS-Ocean by namelist). Maybe this has already been done. I'm not sure we need to do the same for all the GMPAS-JRA1p4* compsets or the GMPAS-IAF* (CORE-II forced) compsets.

@darincomeau
Copy link
Member

This reminds me that we should at some point amend the drof modes to point to the standard runoff files, and remove pointing to the modified runoff files. Not sure if that should be part of this PR or separate.

@cbegeman
Copy link
Contributor Author

@darincomeau Thanks for catching that. I copied your qualification into the description.

@cbegeman
Copy link
Contributor Author

I think we should test that the GMPAS-JRA1p5-DIB-PISMF compset prior to this PR (where removed runoff is handled by pointing to no_rofi_no_rofl runoff files) is BFB with this PR (where runoff is removed in MPAS-Ocean by namelist). Maybe this has already been done. I'm not sure we need to do the same for all the GMPAS-JRA1p4 compsets or the GMPAS-IAF* (CORE-II forced) compsets.

This case from the cryo dev suite is BFB:

NLFAIL SMS_D_Ld1.TL319_IcoswISC30E3r5.GMPAS-JRA1p5-DIB-PISMF.chrysalis_intel.mpaso-jra_1958 (but otherwise OK)

and the cryo dev suite does include a few *GMPAS-DIB-IAF-PISMF* tests that are also BFB

@darincomeau
Copy link
Member

Thanks @cbegeman - after my last comment I think both cases are still pointing to the same modified runoff files with AIS runoff already removed, so while it's good the added namelist is still BFB, it's most likely not actually doing anything, and we want to test that it is BFB when using the original runoff files.

…fied G-case compsets that were using these drof modes
@darincomeau
Copy link
Member

I pushed changes that removed drof modes that pointed to modified AIS runoff forcing files, and modified those G-case compsets that used these now removed drof modes to use standard runoff forcing files.

This removes possible confusion/redundancy of where runoff is being removed in G-case compsets - with this PR it is now strictly handled by mpas-o namelist options (whereas previously the compsets would point to runoff files modified to already remove the AIS runoff).

This will be non-BFB in all G-case compsets that previously used one of these drof modes, since the runoff files are changed. There's about 20 of them, but in discussing with @cbegeman and @xylar we think a number of the compsets using CORE-II or JRA 1p4 can be removed (still available with long compset names) in a separate PR.

@darincomeau
Copy link
Member

I'm running SMS tests on all of the compsets changed here (still going).

I SMS tested GMPAS-JRA1p5-DIB-PISMF here (AIS liquid/solid runoff removed in ocn_comp_mct.F by namelist option) and compared to a baseline generated with master (AIS liquid/solid runoff removed in modified runoff files), and was non-BFB in 9 fields, all related to runoff. All other cpl.hi fields were identical.

@darincomeau
Copy link
Member

Successfully SMS tested the following G-case compsets on chrysalis:

standard G-case compsets

TL319_IcoswISC30E3r5.GMPAS-JRA1p5
TL319_IcoswISC30E3r5.GMPAS-JRA1p4
T62_IcoswISC30E3r5.GMPAS-IAF
T62_IcoswISC30E3r5.GMPAS-NYF

cryo JRA 1p5 compsets

TL319_IcoswISC30E3r5.GMPAS-JRA1p5-DIB-DISMF
TL319_IcoswISC30E3r5.GMPAS-JRA1p5-DIB-PISMF
TL319_IcoswISC30E3r5.GMPAS-JRA1p5-DIB-PISMF-DSGR
TL319_IcoswISC30E3r5.GMPAS-JRA1p5-DIB-PISMF-DSGR-TMIX

cryo JRA 1p4 compsets

TL319_IcoswISC30E3r5.GMPAS-JRA1p4-DIB
TL319_IcoswISC30E3r5.GMPAS-JRA1p4-DIB-DISMF
TL319_IcoswISC30E3r5.GMPAS-JRA1p4-DIB-PISMF
TL319_IcoswISC30E3r5.GMPAS-JRA1p4-DISMF
TL319_IcoswISC30E3r5.GMPAS-JRA1p4-PISMF

cryo CORE-II compsets

T62_IcoswISC30E3r5.GMPAS-DIB-IAF
T62_IcoswISC30E3r5.GMPAS-DIB-IAF-DISMF
T62_IcoswISC30E3r5.GMPAS-DIB-IAF-PISMF
T62_IcoswISC30E3r5.GMPAS-IAF-DISMF
T62_IcoswISC30E3r5.GMPAS-IAF-PISMF
T62_IcoswISC30E3r5.GMPAS-DIB-NYF
T62_IcoswISC30E3r5.GMPAS-DIB-NYF-DISMF
T62_IcoswISC30E3r5.GMPAS-DIB-NYF-PISMF
T62_IcoswISC30E3r5.GMPAS-NYF-DISMF
T62_IcoswISC30E3r5.GMPAS-NYF-PISMF
T62_IcoswISC30E3r5.GMPAS-NYF-PISMF-DSGR

Each case had the correct pair of remove_ais_*_runoff namelist values, and all pointed to standard runoff forcing files in drof.streams.

@cbegeman
Copy link
Contributor Author

Thanks @darincomeau!

@darincomeau
Copy link
Member

I'm now wondering if changing the compsets and removing the drof modes is too much for this PR, but the compsets changed here are the only ones using those drof modes. I can split it up if needed.

@xylar
Copy link
Contributor

xylar commented Oct 31, 2024

I'm finally getting the 2 G-cases (master and this branch merged with master) submitted to test this PR.

I'm running GMPAS-JRA1p5-DIB-DISMF.TL319_IcoswISC30E3r5. I hope that's what we had in mind.

@xylar
Copy link
Contributor

xylar commented Nov 4, 2024

My 2 G-case runs are finished. They are at:

/lcrc/group/e3sm/ac.xylar/scratch/chrys/20241031.GMPAS-JRA1p5-DIB-DISMF.TL319_IcoswISC30E3r5.chrysalis
/lcrc/group/e3sm/ac.xylar/scratch/chrys/20241031.GMPAS-JRA1p5-DIB-DISMF.TL319_IcoswISC30E3r5.mod-so-rof-removal.chrysalis

I'm going to compare them by running MPAS-Analysis in "main vs. control" mode. If we also think a BFB test on specific files would be useful, let me know. I'm not very adept with cprnc but I'm sure I can fumble my way through.

@xylar
Copy link
Contributor

xylar commented Nov 4, 2024

The comparison analysis here:
https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.xylar/analysis/mod-so-rof-removal/20241031.GMPAS-JRA1p5-DIB-DISMF.TL319_IcoswISC30E3r5.main_vs_ctrl/ocean/index.html

As far as I could see, all diffs (also for sea ice analysis) are exactly zero.

@xylar
Copy link
Contributor

xylar commented Nov 4, 2024

@jonbob, if you would like me to cprnc some files, please just let me know (and and remind me what the syntax is).

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'm approving this based on my G-case testing and looking through the code.

@cbegeman
Copy link
Contributor Author

cbegeman commented Nov 4, 2024

@xylar Thanks for your testing!

@jonbob
Copy link
Contributor

jonbob commented Nov 4, 2024

@xylar -- cprnc is fairly easy to use. It just compares two netcdf files, so "cprnc file1 file2". If it complains about the time not being available, then it just needs a "-m" option added. The output should tell you if it thinks the files are identical

@xylar
Copy link
Contributor

xylar commented Nov 5, 2024

Thanks @jonbob. I ran:

/lcrc/group/e3sm/tools/cprnc/cprnc -m \
  20241031.GMPAS-JRA1p5-DIB-DISMF.TL319_IcoswISC30E3r5.chrysalis/run/20241031.GMPAS-JRA1p5-DIB-DISMF.TL319_IcoswISC30E3r5.chrysalis.mpaso.rst.0011-01-01_00000.nc \
  20241031.GMPAS-JRA1p5-DIB-DISMF.TL319_IcoswISC30E3r5.mod-so-rof-removal.chrysalis/run/20241031.GMPAS-JRA1p5-DIB-DISMF.TL319_IcoswISC30E3r5.mod-so-rof-removal.chrysalis.mpaso.rst.0011-01-01_00000.nc

And these 2 final restart files are identical:

SUMMARY of cprnc:
 A total number of     81 fields were compared
          of which      0 had non-zero differences
               and      0 had differences in fill patterns
               and      0 had different dimension sizes
               and      0 had different data types
 A total number of      4 fields could not be analyzed
 A total number of      0 time-varying fields on file 1 were not found on file 2.
 A total number of      0 time-constant fields on file 1 were not found on file 2.
 A total number of      0 time-varying fields on file 2 were not found on file 1.
 A total number of      0 time-constant fields on file 2 were not found on file 1.
  diff_test: the two files seem to be IDENTICAL 

So I believe we're good.

@xylar
Copy link
Contributor

xylar commented Nov 5, 2024

And, yes, cprnc as easy to run once I found it.

Copy link
Member

@darincomeau darincomeau left a comment

Choose a reason for hiding this comment

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

Approve based on excellent developer work and reviewer testing.

@cbegeman
Copy link
Contributor Author

cbegeman commented Nov 5, 2024

Thanks for all your work on this @darincomeau!

@jonbob jonbob added the NML label Nov 12, 2024
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 added a commit that referenced this pull request Nov 12, 2024
)

Update conditions for Southern Ocean ice and river runoff removal

This PR modifies the conditions for river and ice runoff removal in E3SM
cases. This changes the behavior of E3SM cases where data icebergs (DIB)
or ice-shelf melting (ISMF) were active and thus doesn't not affect
water cycle simulations.

River runoff:
  Previous behavior: never removed
  New behavior: removed when atmosphere is not active and ice shelf melt
                fluxes are on
Ice runoff:
  Previous behavior: removed when DIB is on and atmosphere is active
  New behavior: removed when DIB is on

[NML] when DIB or DISMF are active
[BFB] when DIB and DISMF are inactive (all WC configurations)
[non-BFB] when DIB or DISMF are active
@jonbob
Copy link
Contributor

jonbob commented Nov 12, 2024

Passes:

  • ERP_Ld3.ne30pg2_r05_IcoswISC30E3r5.WCYCL1850.chrysalis_intel.allactive-pioroot1

Expected NML and baseline DIFFs for e3sm_cryo_developer:

  • SMS_D_Ld1.TL319_IcoswISC30E3r5.GMPAS-JRA1p5-DIB-PISMF.chrysalis_intel.mpaso-jra_1958
  • ERS_Ld5.T62_oQU240wLI.GMPAS-DIB-IAF-PISMF.chrysalis_intel
  • ERS_Ld5.T62_oQU240wLI.GMPAS-DIB-IAF-DISMF.chrysalis_intel
  • PEM_Ln5.T62_oQU240wLI.GMPAS-DIB-IAF-DISMF.chrysalis_intel
  • PEM_Ln5.T62_oQU240wLI.GMPAS-DIB-IAF-PISMF.chrysalis_intel
  • PET_Ln5.T62_oQU240wLI.GMPAS-DIB-IAF-DISMF.chrysalis_intel
  • PET_Ln5.T62_oQU240wLI.GMPAS-DIB-IAF-PISMF.chrysalis_intel

merged to next

@cbegeman
Copy link
Contributor Author

Thanks for testing @jonbob!

@jonbob jonbob merged commit 2c377c5 into E3SM-Project:master Nov 13, 2024
5 checks passed
@jonbob
Copy link
Contributor

jonbob commented Nov 13, 2024

merged to master and expected NML and baseline DIFFs blessed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mpas-ocean NML non-BFB PR makes roundoff changes to answers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants