User/bgr/homogenize iob#1480
Conversation
- With new option HOMOGENIZE_IOB_FORCINGS = True, a copy of the IOB structure is created with horizontally homogenized fields across all processor elements. - This new feature is primarily for running in single column like configurations with the coupler, which requires perfectly equal forcing across all cells.
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #1480 +/- ##
=========================================
Coverage 29.13% 29.13%
=========================================
Files 235 235
Lines 71061 71061
=========================================
Hits 20707 20707
Misses 50354 50354 Continue to review full report at Codecov.
|
Hallberg-NOAA
left a comment
Hallberg-NOAA
left a comment
There was a problem hiding this comment.
I like the spirit of where this PR is going, and it looks like it should work as written although I do not think that this code will be rotationally invariant. I have two specific questions about the implementation here:
-
Why is there the choice to homogenize the Ice_ocean_boudary_type, rather than the forces and fluxes types? The former requires the creation of a separate structure, and it is specific to the FMS_cap, whereas if the homogenization had been to forcing and mech_forcing types, it could have been done in place, and this capability would have occurred in MOM6/src code that would have been shared across all the various drivers for MOM6.
-
When taking the global means in
homogenize_field(), why not use the order-invariant sums via something like the functionglobal_area_mean()in src/diagnostics/MOM_spatial_means.F90, which would be rotationally invariant?
|
Thanks @Hallberg-NOAA for looking at this and making comments. I will take suggestion in the second comment and update this PR. We can continue the conversation here or offline about your first comment.
|
| allocate(IOB_h%ice_rigidity (isc_bnd:iec_bnd,jsc_bnd:jec_bnd) ) ; IOB_h%ice_rigidity = 0.0 | ||
| call homogenize_field(IOB%ice_rigidity,IOB_h%ice_rigidity, G, index_bounds) | ||
| endif | ||
|
|
There was a problem hiding this comment.
@breichl , what about other IOBs? When I run this code change with "generic tracers" turned on the model crashes with
FATAL: extract_coupler_values: The requested boundary condition is not associated.
Image PC Routine Line Source
MOM6SIS2 00000000026A4A77 Unknown Unknown Unknown
MOM6SIS2 0000000001A421EC mpp_mod_mp_mpp_er 68 mpp_util_mpi.inc
MOM6SIS2 00000000017767FC fms_coupler_util_ 53 FMS_coupler_util.F90
MOM6SIS2 00000000004E20B6 g_tracer_utils_mp 1429 generic_tracer_utils.F90
MOM6SIS2 00000000011995D0 ocean_model_mod_m 551 ocean_model_MOM.F90
MOM6SIS2 00000000013E3A6F MAIN__ 1041 coupler_main.F90
There was a problem hiding this comment.
I think that this issue that Niki is encountering suggests that it might be worth revisiting whether it would be simpler to homogenize the fields in the forcing and mech_forcing types, even if we had to add the option to recalculate the dependent fields (e.g., ustar) based on the primary fields (like taux and tauy). We should not be changing the intent on the various types.
| call get_domain_extent(Ocean_sfc%Domain, index_bnds(1), index_bnds(2), index_bnds(3), index_bnds(4)) | ||
|
|
||
| if (OS%use_homogenized_IOB) & | ||
| call homogenize_IOB(Ice_ocean_boundary, Ice_ocean_boundary_homogenized, OS%grid, index_bnds) |
There was a problem hiding this comment.
@breichl , what is the reason for doing this on an extra copy of IOB? I think if we homogenize the fields on the original Ice_ocean_boundary (rather than the new Ice_ocean_boundary_homogenized ) everything would work fine, including the (generic) tracer fluxes. The problem with Ice_ocean_boundary_homogenized is that it does not have the hidden member "fluxes" allocated.
There was a problem hiding this comment.
One issue with modifying IOB itself is that it is intent(in) in update_ocean_model, I had to make it intent(inout). But after that all seem to have worked fine, including the homogenization and the generic tracers.
There was a problem hiding this comment.
Okay, thanks Niki. I made the copy to avoid modifying this structure since it is intent(in). If we are comfortable making it intent(inout) then this solution works for me. Do the generic tracers fields also need to be homogenized? I'm not sure how difficult that would be or if it is necessary here.
There was a problem hiding this comment.
I am very uncomfortable making the IOB type intent(inout). The intent is there for a reason, as it helps to document which fields are coming from where, and it does so in a way that the compiler can take advantage of for performance.
- This commit uses a MOMr branch code developed by B. Raichl to homogenize the forcings over the column that avoids sst drifts in the physical model. - The code is modified by Niki to make it work for generic tracers. - For more info refer to https://github.com/NOAA-GFDL/MOM6/pull/1480 - Also in this commit are included the parameter changes from B. Reichl for the physical model.
|
@breichl Have you had a chance to review the comments here? |
|
Provided @Hallberg-NOAA thinks #1541 is a better solution I will close this PR |
|
Closing in favor of #1541. |
4 participants
These changes are necessary to run a single-column like configuration in ice-ocean mode. It is difficult to guarantee that the coupler passes perfectly homogeneous fields back to MOM, so it is more straightforward to force the IOB fields to be homogeneous inside of MOM.
Presently there are no test cases that utilize this code, so everything should be unchanged. A new test case will be added shortly to take advantage of this new option in a SCM OM4-like configuration.
@Hallberg-NOAA , looking for some advice if the homogenize_IOB and homogenize_field routines would be better in another module, or if they are okay here.