-
Notifications
You must be signed in to change notification settings - Fork 152
external wave height option #545
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
base: main
Are you sure you want to change the base?
Conversation
Merge CICE-Consortium/Icepack main (v1.5.0) to E3SM-Project/Icepack main
Fixes a bug introduced in V1.5.0. Silicate limitation should not be wrapped in an if statement for iron tracers. nonBFB with active BGC. BFB for all else.
…negative-silicate Corrects negative silicate values in sea ice
…eight add optional wave height input to subroutines in icepack that calculate wave heights
|
I'm thinking of changing the |
|
Thanks for consolidating! I think that looks good. Just a comment on the naming. Now wave_spec_type is only used to determine how wave fracture is computed (convergence over random sea surface height phase, vs one iteration and constant phase). Maybe we should rename 'wave_spec_type' to 'wave_frac_method' or something similar? Then we can later add in new wave fracture methods using this as well. Then we might keep 'wave_spec_type' for the other option (instead of wave_height_type) - as the input data is the wave energy spectrum as a function of frequency, E(f) |
We can certainly generalize this, but I'd prefer to do that in a separate PR from this one.
This would be really confusing, I think. We don't have any new wave data for CICE at the moment, but the input data for |
dabail10
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this looks fine. I am still confused about why dwavefreq goes away.
| wave_sig_ht, & | ||
| wave_spectrum, & | ||
| wavefreq, & | ||
| dwavefreq, & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I am not sure why you are removing dwavefreq from the call, but not wavefreq?
| if (wave_spec) then | ||
| if (wave_sig_ht > puny) then | ||
| call wave_dep_growth (wave_spectrum, & | ||
| wavefreq, dwavefreq, & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. I see here that you removed dwavefreq.
|
|
||
| integer (kind=int_kind) :: k | ||
|
|
||
| w_amp = c2* SQRT(SUM(local_wave_spec*dwavefreq)) ! sig wave amplitude |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Starting to understand now. Is this bfb?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is BFB in my tests so far.
Ah, I hadn't realized that E3SM is using wave significant height rather than the spectrum. Then 'wave_height_type' makes sense. I do think 'wave_frac_method' or 'wave_frac_type' would be less confusing than 'wave_spec_type' for the option to do convergence over random sea surface height phase vs one iteration and constant phase in the wave fracture scheme. |
|
FYI @NoahDay @ezhilsabareesh8 - I don't think this impacts us? (There will be a companion change to cice - see CICE-Consortium/CICE#1071) |
| "", "``random``", "wave data file is provided, sea surface height generated using random number (multiple iterations of wave fracture)", "" | ||
| "``wave_height_type``", "``internal``", "significant wave heights are calculated by icepack from the wave_spectra", "``none``" | ||
| "", "``none``", "No wave data provided, no wave-ice interactions", "" | ||
| "", "``coupled``", "significant wave heights data provided from a coupled wave model, like WW3", "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about the wave spectra? Are these passed from E3SM in addition to significant wave height? Or is there some reconstruction of the wave spectra (e.g. Bretschneider) occurring somewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think they are passing the data to ensure the fields are all consistent. Treating wave height as external forcing makes sense to me since the wave model is a separate component (in E3SM), but sea ice could still modify wave height during the timestep if the coupling needs to be tighter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes- we pass the direction integrated wave spectra for the breaking (controlled by wave_spec_type), as well as wave height (controlled by wave_height_type) .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps for a later PR then, we could make the logic like this?
wave_spec_type
= ‘none’ - No wave data provided, no wave-ice interactions
= ‘profile’ - constant spectra, for testing only
= ‘coupled’ - from file or coupling with wave model
wave_height_type
= ‘none’ ?
= ‘internal’ - calculated from wave spectra within sea ice model
= ‘coupled’ - from file or coupling with wave model
wave_frac_type
= ‘none’ ?
= ‘HT15-single’ - as Horvat & Tziperman (2015) - one iteration with fixed sea surface height phase
= ‘HT15-convergence’ - as Horvat & Tziperman (2015) - run to convergence with random sea surface height phases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. I made a new issue to keep this on the radar. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this idea for for the future!
|
Please run
before final PR. This will update the Icepack interface document as needed. Do a git diff on the doc files to make sure it looks reasonable. Thanks! |
|
The wave-ice parts of this PR has now been fully tested in E3SM. I fully approve of all @eclare108213s consolidations! |
PR checklist
Add option for externally generated significant wave height
@erinethomas @eclare108213
Base_suite is fully BFB with baseline.
Also tested in modified CICE quick_suite, including the 4 FSD tests. All pass, including comparisons with baseline.
A new namelist flag allows significant wave height to be passed into the ice model from a coupler. In addition, this PR
wave_spec_heightout of icepack interface argument lists, since it is initialized viaicepack_init_parameterswave_height_type = 'internal'.This PR is the first step in resyncing the CICE Consortium and E3SM versions of Icepack. It is in draft form until we ensure it runs as expected in E3SM. A few extra commits show up in the list because of differences in histories between the Consortium and E3SM Icepack repositories, going back to the last time they were in sync. Only the wave-height related changes are new, as shown in the file comparison.