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

Disable land ice frazil unless ISMF is on #101

Conversation

cbegeman
Copy link
Collaborator

This PR turns land ice frazil off by default and re-enables it when ice-shelf melt fluxes are enabled in standalone or coupled mode. Land ice frazil is not enabled in data ISMF because frazil fluxes are already included in satellite-derived ISMF products.

@cbegeman
Copy link
Collaborator Author

@jonbob and @darincomeau Can you both take a look at this PR (last 2 commits) when you have a chance and let me know if this seems appropriate to you?

@darincomeau
Copy link
Collaborator

Thanks @cbegeman , I think this approach looks like what we want. I can test it and make sure it's working as intended.

@cbegeman
Copy link
Collaborator Author

@darincomeau That would be great!

@xylar xylar changed the base branch from master to alternate June 12, 2024 07:35
@xylar xylar changed the base branch from alternate to master June 12, 2024 07:35
@xylar
Copy link
Collaborator

xylar commented Jun 12, 2024

This looks right to me!

@darincomeau
Copy link
Collaborator

/lcrc/group/e3sm/ac.dcomeau/scratch/chrys/ERS_Ld5.T62_oQU240wLI.GMPAS-DIB-IAF-DISMF.chrysalis_intel.20240613_165348_cfi0j4/run/mpaso_in: config_frazil_under_land_ice = .false.
/lcrc/group/e3sm/ac.dcomeau/scratch/chrys/ERS_Ld5.T62_oQU240wLI.GMPAS-DIB-IAF-PISMF.chrysalis_intel.20240613_165348_cfi0j4/run/mpaso_in: config_frazil_under_land_ice = .true.
/lcrc/group/e3sm/ac.dcomeau/scratch/chrys/PEM_Ln5.T62_oQU240wLI.GMPAS-DIB-IAF-DISMF.chrysalis_intel.20240613_165348_cfi0j4/run/mpaso_in: config_frazil_under_land_ice = .false.
/lcrc/group/e3sm/ac.dcomeau/scratch/chrys/PEM_Ln5.T62_oQU240wLI.GMPAS-DIB-IAF-PISMF.chrysalis_intel.20240613_165348_cfi0j4/run/mpaso_in: config_frazil_under_land_ice = .true.
/lcrc/group/e3sm/ac.dcomeau/scratch/chrys/PET_Ln5.T62_oQU240wLI.GMPAS-DIB-IAF-DISMF.chrysalis_intel.20240613_165348_cfi0j4/run/mpaso_in: config_frazil_under_land_ice = .false.
/lcrc/group/e3sm/ac.dcomeau/scratch/chrys/PET_Ln5.T62_oQU240wLI.GMPAS-DIB-IAF-PISMF.chrysalis_intel.20240613_165348_cfi0j4/run/mpaso_in: config_frazil_under_land_ice = .true.
/lcrc/group/e3sm/ac.dcomeau/scratch/chrys/SMS_D_Ld1.TL319_IcoswISC30E3r5.GMPAS-JRA1p5-DIB-PISMF.chrysalis_intel.mpaso-jra_1958.20240613_165348_cfi0j4/run/mpaso_in: config_frazil_under_land_ice = .true.
/lcrc/group/e3sm/ac.dcomeau/scratch/chrys/SMS_P512.ne30pg2_r05_IcoswISC30E3r5.CRYO1850.chrysalis_intel.20240613_182655_6j470s/run/mpaso_in: config_frazil_under_land_ice = .true.
/lcrc/group/e3sm/ac.dcomeau/scratch/chrys/SMS_P512.ne30pg2_r05_IcoswISC30E3r5.CRYO1850-DISMF.chrysalis_intel.20240613_180733_yzihnm/run/mpaso_in: config_frazil_under_land_ice = .false.
/lcrc/group/e3sm/ac.dcomeau/scratch/chrys/SMS_P512.ne30pg2_r05_IcoswISC30E3r5.WCYCL1850.chrysalis_intel.20240613_174817_kgaob4/run/mpaso_in: config_frazil_under_land_ice = .false.

Seems to be working as intended - thanks @cbegeman !

Copy link
Collaborator

@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.

Approved based on above testing.

@cbegeman cbegeman marked this pull request as ready for review June 18, 2024 15:21
@cbegeman
Copy link
Collaborator Author

@darincomeau Thanks so much for testing!

@cbegeman
Copy link
Collaborator Author

@jonbob When you have a chance, can you take a look and let me know if this is ready to move over to E3SM? As a reminder, we want this merged so that we don't accidentally have frazil under land ice in our upcoming non-cryo v3 runs.

@jonbob
Copy link
Collaborator

jonbob commented Jun 18, 2024

@cbegeman -- the only thing I'm concerned about is that the logic is in a different namelist group, so it's not getting set in the same section. I don't think it's a big deal, but it might be clearer to duplicate the $OCN_ISMF logic in the frazil_ice namelist section?

@cbegeman
Copy link
Collaborator Author

@cbegeman -- the only thing I'm concerned about is that the logic is in a different namelist group, so it's not getting set in the same section. I don't think it's a big deal, but it might be clearer to duplicate the $OCN_ISMF logic in the frazil_ice namelist section?

Yes, I noticed that. I'm happy to duplicate the logic for clarity. I'll push that change shortly.

@cbegeman cbegeman force-pushed the ocn-disable-land-ice-frazil branch from 3541bfb to decd023 Compare June 19, 2024 01:38
@cbegeman
Copy link
Collaborator Author

@jonbob Your requested changes have been made. Would you like me to do any further testing before migrating to e3sm?

Copy link
Collaborator

@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.

Looks perfect

@jonbob
Copy link
Collaborator

jonbob commented Jun 19, 2024

let's get this to e3sm and I'll merge it as quickly as possible

Copy link
Collaborator

@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 didn't realize I was the hold up here. This is ready to go to E3SM as far as I'm concerned. Thanks for the work on it, @cbegeman!

@cbegeman
Copy link
Collaborator Author

cbegeman commented Jul 5, 2024

Closed in favor of E3SM-Project#6501

Thanks for reviewing @jonbob @darincomeau @xylar!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants