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

Adds internal ocean frazil ice porosity #6802

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

njeffery
Copy link
Contributor

@njeffery njeffery commented Dec 5, 2024

This scheme updates the internal frazil ice salinity redistribution. The default ocean frazil ice parametrization contributes to unphysical stratification of the upper ocean during formation because it uses the ocean-sea ice coupling salinity for internal ocean redistribution.

Changes and simulations are documented here: https://acme-climate.atlassian.net/wiki/spaces/HESF/pages/4820598785/Ocean+Frazil+Ice+Porosity

The code contains comments !NJ to help explain the changes. I can clean that up once the code has been review.

[NML]
[non-BFB]

These changes assume frazil ice retains salt until precipitating at the surface.

Treatment under land ice remains the same.

nonBFB
Adds namelist field config_frazil_ice_porosity
Modifies the internal frazil salinity

nonBFB
Depth of maximum frazil determined by config_frazil_maximum_depth
Uses config_frazil_ice_porosity to compute internal frazil salinity

nonBFB
@njeffery njeffery assigned njeffery and jonbob and unassigned njeffery Dec 5, 2024
@njeffery njeffery added the non-BFB PR makes roundoff changes to answers. label Dec 5, 2024
Copy link

github-actions bot commented Dec 5, 2024

PR Preview Action v1.4.8
🚀 Deployed preview to https://E3SM-Project.github.io/E3SM/pr-preview/pr-6802/
on branch gh-pages at 2024-12-05 17:47 UTC

@xylar xylar requested review from cbegeman and xylar December 5, 2024 17:49
@xylar
Copy link
Contributor

xylar commented Dec 5, 2024

Relevant discussion can be found at:
E3SM-Ocean-Discussion#115

@xylar
Copy link
Contributor

xylar commented Dec 5, 2024

@njeffery, you mentioned that you had a design document on your to-do list. Could it maybe make sense to hold off on this PR until that is done?

E3SM-Ocean-Discussion#115 (comment)

@xylar
Copy link
Contributor

xylar commented Dec 5, 2024

Aach, sorry! I missed that that was linked above!

@xylar
Copy link
Contributor

xylar commented Dec 5, 2024

@njeffery, how easy or hard would it be to run a model vs. model run of MPAS-Analysis so we could see the difference plots between your run (without BGC) and the control run? The differences are definitely there but they're subtle enough that those difference plots might really help.

I'd be happy to help with running the analysis if need be. The main thing would be whether the analysis output (not just the plots) is still available for the control run.

@njeffery
Copy link
Contributor Author

njeffery commented Dec 5, 2024

I have the first 100 years of the control run ocean history files only on Perlmutter: /global/cfs/projectdirs/e3sm/njeffery/E3SMv3/20231209.v3.LR.piControl-spinup.chrysalis/archive/ocn/hist . I could bring the ice history directory too. What else would you need?

@xylar
Copy link
Contributor

xylar commented Dec 5, 2024

@njeffery, I think I would want everything on Chrysalis because that's where the control run is. Since you ran on Anvil, it's already there. I can give it a try tomorrow. The config files from your analysis run and Wuyin's should tell me where everything is (or was at least) and I can let you know if anything is missing.

@xylar
Copy link
Contributor

xylar commented Dec 6, 2024

@njeffery, okay, I see the issue now. This run is no longer on Chrysalis. It is pretty difficult to compare runs on different machines so I think we would either need those 100 years of ocean and sea ice history, plus a restart file for ocean and sea ice, plus the namelist and streams files. Or we would need the MPAS-Analysis run that was already done (maybe easier). I will see if I can restore that with globus directly from tape at NERSC.

Update:

  1. the analysis data that we need isn't in the archive. That's not really a bad thing because it was generated with an older version of MPAS-Analysis so better to generate both main and control analysis with the same version.
  2. I wasn't able to zstash extract directly to Chrysalis (maybe that's what you found) so I'm extracting on Perlmutter and then I'll globus them over to Chrysalis. I likely won't get the analysis going until Monday because I'm away for the weekend.

@xylar xylar requested a review from darincomeau December 6, 2024 11:48
@xylar
Copy link
Contributor

xylar commented Dec 6, 2024

@cbegeman, @darincomeau and @proteanplanet, I know you are both going to be busy with AGU but I wanted to check with you about this. I would not feel comfortable approving this change, which I think could be significant, without a B-case simulation with thermodynamics active in the ice-shelf cavities (PISMF - prognostic ice-shelf melt rates). I think this might mean doing a simulation branched of from @darincomeau's alfred1 simulation.

@xylar
Copy link
Contributor

xylar commented Dec 6, 2024

@njeffery, I now see that you've moved the first 100 years of data from your simulation to Perlmutter. I can do the analysis there then (on Monday).

Could you give me permission to read the files and directories in the archive on your scratch drive?

chmod -R go+rX /global/cfs/projectdirs/e3sm/njeffery/E3SMv3/20241010.Frazil2.underIce.piControl.anvil

I'll do the same for my data once it's extracted. It's at:

/pscratch/sd/x/xylar/20231209.v3.LR.piControl-spinup.chrysalis

@njeffery
Copy link
Contributor Author

njeffery commented Dec 6, 2024

Thanks, @xylar. You should now have permission.

@proteanplanet
Copy link
Contributor

@xylar and @njeffery: This is what has to happen to this PR from here in: It requires a B-case LR 100-year simulation to be compared with a control. Moreover, the change must be introduced initially to master as a BFB change. So the PR gets tested with the change to porosity to the value currently set in the code for this PR, but the code should be set with porosity at zero which I assume makes it BFB with master. Then, for the time being, we test, for example, SORRM and BGC configurations with the porosity set to non-zero in the namelist changes in run scripts, also the HR group, who are interested in testing this as well. Once we are all happy it's performing correctly, we then submit another PR making this default after broad consultation in group meetings.

@proteanplanet
Copy link
Contributor

Documentation at the link provided by @njeffery meets requirements.

@xylar
Copy link
Contributor

xylar commented Dec 6, 2024

@proteanplanet

Moreover, the change must be introduced initially to master as a BFB change. So the PR gets tested with the change to porosity to the value currently set in the code for this PR, but the code should be set with porosity at zero which I assume makes it BFB with master.

I thought this initially as well but reading the code I think a porosity of zero would only be BFB under ice shelves, where the current salinity of ice is zero. In the open ocean, this is non-BFB in all conditions.

@vanroekel
Copy link
Contributor

I agree with @xylar in the open ocean the frazil reference salinity is used right now, so porosity of zero wouldn't be BFB in the open ocean. I also don't believe we can use the code as and just tuning the porosity to get BFB.

@njeffery
Copy link
Contributor Author

njeffery commented Dec 6, 2024

Do you want me to add a config flag that adds an option which reverts to the original code?

@proteanplanet
Copy link
Contributor

Sorry I was unclear, I'm requesting either a switch to make this BFB, which could be, for example, setting porosity to zero, or a separate flag. We are doing something similar for changes to top of atmosphere radiation for CMIP7, which will be highly likely to become the default, in a separate PR later.

@njeffery
Copy link
Contributor Author

njeffery commented Dec 6, 2024

Sure. i can make it a stealth feature.

@cbegeman
Copy link
Contributor

cbegeman commented Dec 6, 2024

One option, which may be along the lines that @proteanplanet was thinking, would be to have a config option that is the minimum frazil salinity, defaulting to 4, and then we could reproduce the current behavior of the model by setting frazil ice porosity to 0 (minimum would kick in everywhere).

Comment on lines +992 to +995
<nml_option name="config_frazil_ice_porosity" type="real" default_value="0.85" units="1"
description="Assumed porosity of frazil"
possible_values="Any positive real number from 0 to 1."
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if we should call this something other than porosity. When I hear porosity, I assume that the volume of frazil ice formed is only porosity * frazil ice thickness tendency (* areaCell) because part of the frazil ice tendency (the volume advected upward) is pure seawater. But my understanding of the implementation is really that this is expressing the salinity of the frazil as a fraction of the surrounding seawater (the ice formed and latent heat extracted is the full frazil ice thickness tendency). So I wonder if a less confusing name would be something like config_frazil_ice_salinity_fraction.

Copy link
Contributor

Choose a reason for hiding this comment

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

I also got a bit confused by the use of "porosity". I think the use of that term was partly responsible for the confusion we had in our webex discussion a little while back. I seemingly couldn't let go of the idea that not just salt but seawater was being captured and transported by frazil in this approach.

@xylar
Copy link
Contributor

xylar commented Dec 9, 2024

A quick update on MPAS-Analysis: I have things all set on Perlmutter. I have submitted jobs but they're sitting in the queue. I'll post links here once they've run, hopefully tomorrow.

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.

6 participants