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

+(*)Fix dimensional rescaling with HOMOGENIZE_FORCINGS #78

Merged
merged 4 commits into from
Mar 13, 2022

Conversation

Hallberg-NOAA
Copy link
Member

This PR adds new optional arguments to 7 routines, and then uses them to
correct the dimensional rescaling when HOMOGENIZE_FORCINGS = True. All answers
in the existing MOM6-examples test suite are bitwise identical, but there are
revisions to public interfaces. This PR addresses MOM6 issue #60, which can be
closed once this PR is accepted. The commits in this PR include:

  • fa2832c11 +(*)Rescale variables with homogenize_forcing
  • b3e80f73f +(*)Add tmp_scale arguments to global_area_mean

  Added new tmp_scale optional arguments to global_area_mean, global_layer_mean,
global_area_mean_u, global_area_mean_v, global_area_integral, global_volume_mean
and global_mass_integral.  This argument allows the reproducing sums to work
with rescaled variables without undoing the scaling of the returned variable.
Also corrected the area_rescaling in global_area_mean_u and global_area_mean_v,
which were substantially changing answers when horizontal distances were being
rescaled (i.e., if L_RESCALE_POWER is not 0).  These routines had only been
added recently, so this change is only changing answers with HOMOGENIZE_FORCINGS
= True.
  Add optional tmp_scale arguments to the various homogenize_field routines and
use them in numerous calls in homogenize_forcing and homogenize_mech_forcing, so
that the answers will pass the dimensional rescaling tests for large rescaling
factors when HOMOGENIZE_FORCINGS = True.  Among other changes is the addition of
a verticalGrid_type argument to homogenize_forcing.  All answers are bitwise
identical in the existing MOM6-examples test suite, but it will change answers
in other cases with dimensional rescaling active and homogenized forcing.
@Hallberg-NOAA Hallberg-NOAA linked an issue Feb 27, 2022 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Feb 27, 2022

Codecov Report

Merging #78 (92f5a98) into dev/gfdl (3042942) will increase coverage by 3.23%.
The diff coverage is 9.09%.

Impacted file tree graph

@@             Coverage Diff              @@
##           dev/gfdl      #78      +/-   ##
============================================
+ Coverage     25.82%   29.05%   +3.23%     
============================================
  Files           244      244              
  Lines         71808    71930     +122     
============================================
+ Hits          18543    20898    +2355     
+ Misses        53265    51032    -2233     
Impacted Files Coverage Δ
src/core/MOM.F90 58.44% <0.00%> (+0.50%) ⬆️
src/core/MOM_forcing_type.F90 43.08% <0.00%> (+0.02%) ⬆️
src/diagnostics/MOM_spatial_means.F90 15.70% <21.27%> (+0.81%) ⬆️
src/ALE/PPM_functions.F90 56.75% <0.00%> (-10.99%) ⬇️
src/ALE/MOM_remapping.F90 47.21% <0.00%> (-0.35%) ⬇️
src/ALE/MOM_regridding.F90 21.36% <0.00%> (-0.10%) ⬇️
...c/parameterizations/vertical/MOM_vert_friction.F90 42.54% <0.00%> (-0.04%) ⬇️
src/ALE/regrid_edge_values.F90 20.41% <0.00%> (+0.08%) ⬆️
src/core/MOM_density_integrals.F90 9.37% <0.00%> (+0.11%) ⬆️
...ig_src/drivers/solo_driver/MOM_surface_forcing.F90 21.05% <0.00%> (+0.12%) ⬆️
... and 43 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3042942...92f5a98. Read the comment docs.

Copy link

@breichl breichl left a comment

Choose a reason for hiding this comment

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

I agree that these changes make sense and are needed for the scaling tests. I have tested the changes in a sample case that I have and found that there is a slight change in answers. However, the change is due to an errant second call of the homogenization step for the lprec field (L3559 and L3560 was identical in the old version of the code). Therefore, the new code is correct and I approve these changes.

@Hallberg-NOAA Hallberg-NOAA added the bug Something isn't working label Mar 12, 2022
@adcroft adcroft self-assigned this Mar 12, 2022
@adcroft
Copy link
Member

adcroft commented Mar 12, 2022

Copy link
Member

@adcroft adcroft left a comment

Choose a reason for hiding this comment

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

@adcroft adcroft merged commit 6e85b23 into NOAA-GFDL:dev/gfdl Mar 13, 2022
@Hallberg-NOAA Hallberg-NOAA deleted the fix_homogenize_forcing branch March 22, 2022 14:03
marshallward pushed a commit that referenced this pull request May 3, 2022
update to MOM6 main branch 20211019 commit
marshallward added a commit that referenced this pull request May 3, 2022
Note that most of these commits are from previously squashed pull
requests, and this PR is restoring them.

- 6360dbb Merge branch 'main' into main_to_dev
- bac8031 Merge pull request mom-ocean#1566 from jiandewang/EMC-FMS-mixed-mode-20220411
- e532d86 Merge pull request #88 from marshallward/missing_attrib_with_class_bugfix
- d380f1d An alternate fix to class(*) issues with FMS 2022-01
- 8ecf333 Merge pull request #87 from jiandewang/feature/update-to-main-20220317
- ba37f94 Merge remote-tracking branch 'FSU/main' into feature/update-to-main-20220317 this is corresponding to MOM6 main 20220317 commit (hash # 399a7db)
- 44313d9 Merge pull request #85 from jiandewang/feature/update-to-main-20220217
- 966707f Merge remote-tracking branch 'GFDL/main' into feature/update-to-main-20220217 this is corresponding to MOM6 main branch 20220217 commit (hash # 6f6d4d6), which originally based on GFDL-candidate-20220129
- 32c0e1e Merge pull request #81 from jiandewang/feature/update-to-main-20211220
- 9642b1d delete external/OCEAN_stochastic_phyiscs directory as Phil re-coded in external/stochastic_physics directory
- e7c9ada solve minor conflict in mom_cap.F90 mom_ocean_model_nuopc.F90 and MOM_energetic_PBL.F90, add two new files: src/parameterizations/stochastic/MOM_stochastics.F90 and config_src/external/stochastic_physics/stochastic_physics.F90
- 90d5961 Merge pull request #78 from jiandewang/feature/update-to-GFDL-20211019
- fd02017 Merge remote-tracking branch 'GFDL/main' into feature/update-to-GFDL-20211019
- 36f17eb Merge pull request #72 from pjpegion/ocn_stoch_july2021
- a9a957e return a more accurate error message in MOM_stochasics
- 56bb41e Merge branch 'ocn_stoch_july2021' of https://github.com/pjpegion/MOM6 into ocn_stoch_july2021
- ca2ae1c update to dev/emc
- 14ca4a1 Merge pull request #76 from jiandewang/feature/update-to-GFDL-20210914
- 29016c2 Merge remote-tracking branch 'GFDL/main' into feature/update-to-GFDL-20210914 merge GFDL main 20210914 commit (hash # c09e199)
- a8577df Merge branch 'NOAA-EMC:dev/emc' into ocn_stoch_july2021
- f8a8e4c update to gfdl 20210806 (#74)
- 16e6af0 update to dev/emc
- 237a510 add comments
- 1b4273d revert logic wrt increments
- 5b2040e add logic to remove incrments from restart if outside IAU window
- c5f2b72 add write_stoch_restart_ocn to MOM_stochastics
- bdf2dc7 doxygen cleanup
- 8bc4acc move stochastics to external directory
- a3fa3a1 Merge remote-tracking branch 'upstream/dev/emc' into ocn_stoch_july2021
- e4bc007 stochastic physics re-write
- 202cbd4 update to dev/emc
- 61717ee Merge remote-tracking branch 'origin/dev/emc' into ocn_stoch
- 565e0bb remove debug statements
- a4c0411 Merge remote-tracking branch 'upstream/dev/emc' into ocn_stoch
- 689a73f remove PE_here from mom_ocean_model_nuopc.F90
- 8afe969 clean up of mom_ocean_model_nuopc.F90
- 25ed4fc revert MOM_domains.F90
- b8d9888 place stochastic array in fluxes container and make SPPT specific arrays allocatable
- d984a7e remove stochastics container
- eb88219 clean up of code for MOM6 coding standards
- 6e3ea1b correct coupled_driver/ocean_model_MOM.F90 and other cleanup
- 0b99c1f make stochastics optional
- 85023f8 Merge remote-tracking branch 'upstream/dev/emc' into ocn_stoch
- 80f9f44 clean up MOM_domains
- 5443f8e remove blank link in MOM_diagnostics
- 1727d9a re-write of stochastic code to remove CPP directives
- 600ebf9 Merge remote-tracking branch 'upstream/dev/emc' into ocn_stoch
- 6bb9d0b fix non stochastic ePBL calculation
- 1d7ffa3 clean up code
- 040e1f1 Merge pull request #13 from NOAA-EMC/dev/emc
- 2cba995 Merge branch 'dev/emc' into ocn_stoch
- 1dc0f4f Merge remote-tracking branch 'upstream/dev/emc' into dev/emc
- 4bd9b9e clean up debug statements
- 25ed5ef additions for stochy restarts
- a2a374b add stochy_restart writing to mom_cap
- 0c15f4c Update MOM_diabatic_driver.F90
- 167a62e Merge pull request #12 from pjpegion/dev/emc
- bd477a9 Update MOM_diabatic_driver.F90
- 7212400 Update MOM_diabatic_driver.F90
- 7de295c cleanup of code and enhancement of ePBL perts
- cd06356 Merge pull request #11 from NOAA-EMC/dev/emc
- 9896d61 Merge pull request #9 from pjpegion/dev/emc_merge
- 0a62737 Merge branch 'ocn_stoch' into dev/emc_merge
- 3cad1ba Merge pull request #8 from NOAA-EMC/dev/emc
- c2aa2a8 updates from dev/emc
- 182ef34 additions for stochastic physics and ePBL perts
- 671c714 Merge pull request #1 from NOAA-EMC/dev/emc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HOMOGENIZE_FORCINGS does not rescale properly for large factors
3 participants