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

dynamic ice shelf #1338

Merged
merged 20 commits into from
Mar 18, 2021
Merged

Conversation

OlgaSergienko
Copy link
Contributor

@OlgaSergienko OlgaSergienko commented Feb 24, 2021

Several bugs were corrected in MOM6_ice_dynamics.F90. MOM6_ice_shelf.F90 runs with DYNAMIC_SHELF_MASS = TRUE
Two test cases exercise the dynamic ice-shelf code with MOM6 rho
/lustre/f2/dev/gfdl/Olga.Sergienko/MOM6expls/ocean_only/ice_shelf_solo/IS2Dincl_plane_rho/
and layer mode
/lustre/f2/dev/gfdl/Olga.Sergienko/MOM6expls/ocean_only/ice_shelf_solo/IS2Dincl_plane_layer/

@codecov
Copy link

codecov bot commented Feb 24, 2021

Codecov Report

Merging #1338 (5839494) into dev/gfdl (8dd9072) will decrease coverage by 0.09%.
The diff coverage is 0.00%.

❗ Current head 5839494 differs from pull request most recent head edc15f6. Consider uploading reports for the commit edc15f6 to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##           dev/gfdl    #1338      +/-   ##
============================================
- Coverage     45.82%   45.73%   -0.10%     
============================================
  Files           234      234              
  Lines         72667    72821     +154     
============================================
  Hits          33302    33302              
- Misses        39365    39519     +154     
Impacted Files Coverage Δ
src/ice_shelf/MOM_ice_shelf.F90 0.00% <0.00%> (ø)
src/ice_shelf/MOM_ice_shelf_dynamics.F90 0.00% <0.00%> (ø)
src/ice_shelf/MOM_ice_shelf_initialize.F90 0.00% <0.00%> (ø)
...rc/parameterizations/vertical/MOM_tidal_mixing.F90 9.63% <0.00%> (ø)
...eterizations/lateral/MOM_lateral_mixing_coeffs.F90 59.76% <0.00%> (ø)
...arameterizations/lateral/MOM_thickness_diffuse.F90 64.23% <0.00%> (+0.26%) ⬆️

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 8dd9072...edc15f6. Read the comment docs.

@marshallward
Copy link
Collaborator

@OlgaSergienko Can you look into some of the issues here? I'm seeing several trailing whitespace and line length errors, as well as a few undocumented variables.

https://github.com/NOAA-GFDL/MOM6/pull/1338/checks?check_run_id=1974152973

@Hallberg-NOAA
Copy link
Collaborator

Given that this PR includes extensive changes to a portion of the code, this PR should come with a more thorough description of what was done in the PR as a whole, why it was done, how it was tested (including which tests are passing or failing), and under what conditions changes should be expected to either solutions or output.

@marshallward
Copy link
Collaborator

There are many blocks of commented code in this PR, and it's not clear why they need to remain. Is it possible to remove them?

@OlgaSergienko
Copy link
Contributor Author

There are many blocks of commented code in this PR, and it's not clear why they need to remain. Is it possible to remove them?

It is possible. What other changes are required? Suggestions/recommendations are welcomed and appreciated.

@Hallberg-NOAA
Copy link
Collaborator

This PR has passed pipeline testing at https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/12278.

Copy link
Collaborator

@Hallberg-NOAA Hallberg-NOAA left a comment

Choose a reason for hiding this comment

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

I have examined these revised changes, and I agree with these code modifications.

@Hallberg-NOAA Hallberg-NOAA merged commit 3193ab0 into mom-ocean:dev/gfdl Mar 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants