package iceplume - reopening #682
Conversation
- Overhaul of hardcoded constants, some redefined using mitgcm const. - Fix bug related to assumption of glacial front geometry and dxg,dyg - Adopt mitgcm convention to convert between freshwater and mass. - Add global variables specific to addMass from plume processes, (subglacial and background (bg)) - Note bug of resetting global addMass (but not fix yet) - Add diagnostics for global addMass as well as subglacial and bg - Modify where iceplume fields are initialized between iceplume_init_varia and iceplume_calc as precursor to determine which are global vs local - Remove ability to read in combinations of conflicting inputs w_sg and r_sg at the same time. - Minor fix of wrong msg printed in iceplume_init_fixed.F - From changes in constants and conversions, results of T/S at grid point of plume of initial strength 8 m3/s changed by approximately 1e-3 for T and 1e-6 for S after 8 timesteps.
- Separate iceplume into its own ifdef block in external_forcing.F - Overhaul of glacier geometry to remove hardcoded dependency (erroneosly) on model dxg,dyg with introduction of variables dLnormal (old delta_x), dLtangential (old delta_y), hVelCell_tangential (old uVelCell and vVelCell) and new GlacierFront_is_NorthSouth logical flag. - Remove known bug that erroneosly reset global addMass field - Complete removal of rhoShelfIce, replace with rUnit2Mass - Add more diagnostics to iceplume_diagnostics_init.F - None of these mods changed result of atn's test case.
- Change variable names to (1) avoid confusion between angle (in deg)
and model potential temperature (theta): theta_sg becomes Angle_sg_0;
(2) distinguish time-varying variables (w_sg, r_sg) vs fixed to be
read in from namelist (T_sg_0, S_sg_0, Angle_sg_0)
- Partial disable of ability to read in from namelist time-changing
variables w_sg, r_sg (in progress, to be fully removed at next mod).
- Remove duplicates of initialization of 3D fields in iceplume_calc.F
as they have been already done in iceplume_init_varia.F
- Comment out potential erroneous init of plume T/S to model T/S
(not sure about this, left as question)
- No change in atn's test case.
- Large overhaul of local 3D arrays inside iceplume_calc.F vs global as defined in ICEPLUME.h and initialized in iceplume_init_varia.F. Result: (1) Removal of majority of global 3D arrays defined in ICEPLUME.h, (2) removal of many "local" arrays inside iceplume_calc that were defined purely for diagnostic purposes. - Edit inputs to diagnostic calls of global array in iceplume_calc.F to enable future avoidance of defining "local" arrays - Fully disable reading in time-varying variables Q_sg,w_sg,r_sg from namelist in iceplume_readparms.F - Initialize time-varying variables in iceplume_init_varia.F instead of preset in iceplume_readparms.F - Add comments on initialization - None of these mods change the results of atn's test case.
- Add overlap to iceplume_tendency_apply_[t,s] function and call to them inside apply_forcing.F and external_forcing.F - Add diagnostics for [temp,salt]*addmass3dplume inside apply_forcing.F and update list of diagnostics init in diagnostics_main_ini.F - Update variable names to appropriate iceplume pkg, e.g., icefront_tendT is now iceplumeBG_tendT, to avoid confusion when pkg/icefront is used. Relatedly add "bg" when appropriate add "BG" to distinguish background melt from main subglacial discharge plus entrainment plume melt. - Clean up iceplume_calc.F (initialization/reset) and ICEPLUME.h - No change in results of atn's small test case
- Major change to read in only one field runoffQsg (Q_sg) instead of two fields runoffVel (w_sg) and runoffRad (r_sg). This is because, based on Tom Cowton's intention, a rate Q_sg is chosen, based on which w_sg is assumed to be 1.0 and r_sg is computed OFFLINE in matlab, then output to two 4-D arrays runoff[Vel,Rad], which subsequently is then read in and re-combined to produce runoffQsg (Q_sg) online in iceplume_calc.F . By making this mod, we remove the extra offline workload and hidden assumption that w_sg is assumed to be 1, and potential error if the offline and online conversions of Q_sg<-->[w,r]_sg happen to differ. - Add a new fixed variable wVel_sg_0, set to 1.0, to explicitly show the assumption of velocity, and for use to compute r_sg = runoffRad online. Online runoffVel = w_sg = wVel_sg_0. - iceplume_tendency_apply: only cosmetic change here, not relevant to the Q_sg switch. - No change in results of atn's small test.
- Enable reading of Q_sg through pkg/exf
- exf_getffields.F: add ifdef iceplume block and prep Q_sg field file
and properties
- ICEPLUME.h: add ifdef block allow_exf and params relevant to Q_sg
field properties, change Q_sg from RS to R
- iceplume_init_fixed.F: add ifdef allow_exf, call EXF_GETFFIELD_START
- iceplume_readparms.F: add ifdef allow_exf and allow_exch2,
add new namelist parm04 relevant to Qsg field properties
- iceplume_diagnostics_init.F, iceplume calc.F: cosmetic change,
not related to switch to exf.
- No change in results of atn's small test.
…os char * remove dos char return in ICEPLUME.h and ICEPLUME_OPTIONS.h * shorten definition of arrays so that expansion from _RL to real*8 will not exceed 72char * add if/def block around allow_exf in iceplume_readparms.F and allow_addfluid in iceplume_calc.F * add implicit none and def of variables in opkda[1,2].F to enable compilation * one outstanding issue left not fixed: conflict of array definitions between call dprep (passing in real*8 array) versus definition of same array as integer inside subroutine dprep, all inside opkda1.F
|
This is a re-opening of an accidentally deleted PR #652. For testing, i created a PR in verification_other, MITgcm/verification_other#58 . Once this set up is copied into verification, or put into MITgcm/verification_other/ level , it should compile and run successfully with this PR #(was 652, now) 682. |
|
@antnguyen13 I run the small test experiment you provided (MITgcm/verification_other#58 ) and this is nice to have an example using pkg/iceplume. |
|
@antnguyen13 Any update on this ?
I tried again after merging master branch in (including pr 666) and got the same error. |
- fix overlap bug to 2D arrays in iceplume_calc reported by M.Wood - fix out-of-bound error in 1D vertical array related to iceDepthK - move dLtangential init to after dependent variables are defined - remove not-needed runoff[Vel,Rad] 2D arrays in iceplume_calc
|
@jm-c i've now fixed the out-of-bound bug, along with a few more bugs (that also crashes the code once you get through the bug you have reporeted) related to overlap and initialization issues reported by a couple of other users (M.Wood and K.Schulz). In addition, I've gone ahead and merge important code updates reported to me by M.Wood and K.Schulz, as well as implemented K.Schulz 2022 parameterization . These changes did NOT change any results in the verification_other/iceplume_testcase , yet at the same time they'll allow both M.Wood and K.Schulz to track any suggested modifications from now on on either their published or currently in-progress set ups. |
… to top of subroutine
- Rename backgroundVel to backgroundVelThresh - Make iceplume_meltrate.F to output in m/s instead of m/day to enable a general call - Consolidate FOUR duplicate chunks of code inside iceplume_plume_model.F into a single call to existing subroutine iceplume_meltrate.F. - Add significantly more comments as well as change variable names in iceplume_meltrate.F and iceplume_plume_model.F to make variables and calculations easier to follow. - Change confusing subroutine ''Jenkins'' to ''sheetplume'' as it is intended to be in iceplume_plume_model.F. - These mods did not change results of iceplume_testcase. The mods are made to enable soon-to-be merged of code from Schulz et al., 2022.
There was a problem hiding this comment.
This a review for this pkg interface, I did not check the inner part (plus I am little bit afraid by the 18.k lines of code in the 3 opkd*.F files):
- The main issue is related to unsetting
temp_addMassandsalt_addMass. I put a comment below and will give more details about the reasoning around this. - the pieces of code to load the glacial discharge (runoffQsg), currently in
external_fields_load.Fandexf_getffields.F, could be moved into a pkg/iceplume S/R, called dirrectly fromload_fields_driver.F. Might need a specific IcePlumeLoadedRec for the bits outside exf, or just use the old way ("IF ( intime0.NE.intimeP )". - regarding
ICEPLUME.h, it would be useful to have 1 line of description for each variable there. And would be much better without the 2 included CPP header files and without includingPTRACERS_SIZE.h. Might think of splitting this file into "ICEPLUME_PARAMS.h" and "ICEPLUME_FIELDS.h" (or "_VAR.h") (and may be just one for IcePlume & pTracers if needed ?) to same few files from including EXF_OPTIONS.h and PTRACERS_SIZE.h. - there is not need to make any changes in
external_forcing.F, this file is just kept for backward compatibility (USE_OLD_EXTERNAL_FORCING) with no new addition. - It would be good to add the Cowton etal JGR paper to the bib file and at least the title of this new pkg (with this reference) as a starting point in the documentation.
- I am not completely sure about filling "addMass " diagnostic from
diagnostics_fill_state.F, instead of fromdiags_oceanic_surf_flux.F. - I am not convinced by the use of an additional local "tmpVar" in
apply_forcing.F, and did not find where the new main model diagnostics "tAddMass" and "sAddMass" are filled when pkg/iceplume is not compiled.
model/src/ini_parms.F
Outdated
| C-- For backward compatibility, set temp_addMass and salt_addMass | ||
| C to temp_EvPrRn and salt_EvPrRn if not set in parameter file "data" | ||
| IF (temp_addMass .EQ. UNSET_RL) temp_addMass = temp_EvPrRn | ||
| IF (salt_addMass .EQ. UNSET_RL) salt_addMass = salt_EvPrRn | ||
| #else | ||
| temp_addMass = UNSET_RL | ||
| salt_addMass = UNSET_RL |
There was a problem hiding this comment.
I think resetting to UNSET_RL could quickly become a problem, even more for salt_addMass, as we are working on spreading river run-off in the vertical, when using fine vertical res. near the surface.
Practically, it will swich river run-off to salty water (same salinity as ocean) right away.
And for temp_addMass, it would prevent to use the Energy Reference Level (ERL) when compiling iceplume pkg. Since using the ERL is the recommended way of dealing with multiple components exchanging fluid, this is also a major limitation.
Could we find a way to work with default salt_addMass and temp_addMass set to ERL ?
Now if this is not possible, we would still not reset these setting but instead check and stop if not set for pkg/iceplume to work.
|
I am also seeing some unexpected changes for diagnostic "IPmasspl" when using more than 1 tile per proc (i.e, nSx > 1): I run the |
…lude accordingly - ICEPLUME_PARAMS.h: paramemters from namelists and general usage - ICEPLUME_FIELDS.h: multi-dimensional fields - ICEPLUME_EXF_INTERP.h: fields used within pkg/exf - ICEPLUME_PTRACERS_FIELDS.h: fields used within pkg/ptracers Merge of namelist parm02 parameters into parm01. Fields related to pkg/ptracer now in namelist parm02. Fields related to pkg/exf now in namelist parm03.
|
@jm-c I've done a overhaul of this! except for the most important point 1. related to resetting [temp,salt]_addMass, which I'd like to now connect with you about (perhaps in a meeting offline?) Regarding the rest of your points, let me address one by one here:
Lastly, i've also updated the verification_others/iceplume_test with 2 changes: make SIZE.h with 2 tiles in x-dir to test multiple tiles, and change data.iceplume namelists accordingly to follow updates above in pkg/iceplume namelists. |
- iceplume_calc.F:
- Now calculating and passing out addmass, heat, salt fluxes for
both subglacial discharge (pl) and background submarine melt (bg).
- Remove intermediate 1D profiles of intermediate confusing units
and signs
- switch fluxes from g/kg/s /m2 and W/m2 to g/kg/s and W
This switch creates very small within machine-precision difference
- switch melt rate unit from m/day to m/s to avoid constant carrying
around of the factor 1/secInday or secInday
- Another minor change that cause within machine-precision difference:
1. _d 0/twoRL to *HalfRL
- remove extra definition of delta_z, which is the model drF.
- apply_forcing.F: now use heat and salt fluxes to compute tend for plumes
- iceplume_tendency_apply.F: now use heat and salt fluxes to compute tend for background melt
- iceplume_init_varia.F: initialize global heat,salt,addmass arrays
- ICEPLUME_FIELDS.h: add global heat,salt,addmass arrays where required.
remove 1D arrays local only to iceplume_calc.F
|
@jmc I've made a first attempt to switch to flux form for calculations of tendency. The important points are:
While doing the calc of fluxes, i saw how the original code was done and got also a bit confused on why while for the submarine background melt (bg) we I think the current switch is incomplete. I kept it such that old results are duplicated to within machine precision. But once I have more inputs from you as to whether what i'm doing is making sense or not, i can proceed with deleting some un-needed code. |
|
@antnguyen13 I have few preliminary comments:
|
What changes does this PR introduce?
Introduce a new package "iceplume" to parameterize melt plumes at glacier fronts as well as code in main branch that propagate the properties computed from the pkg to state variables.
What is the current behaviour?
There is no current behavior. This commit submit a new pkg.
What is the new behaviour
A new package to parameterize melt plumes at glacier fronts
Does this PR introduce a breaking change?
It should not, because it is a new package.
Other information:
Reference: Cowton, T., D. Slater, A. Sole, D. Goldberg, and P. Nienow (2015), Modeling the impact of glacial runoff on fjord circulation and submarine melt rate using a new subgrid-scale parameterization for glacial plumes, J. Geophys. Res. Oceans, 120, 796–812, doi:10.1002/2014JC010324.
Suggested addition to
tag-index(To avoid unnecessary merge conflicts, please don't update
tag-index. One of the admins will do that when merging your pull request.)