Skip to content

Adding metric terms to account for non-rectangular shape of grid cells#1079

Merged
alex-huth merged 2 commits intoNOAA-GFDL:dev/gfdlfrom
OlgaSergienko:metrics
Apr 20, 2026
Merged

Adding metric terms to account for non-rectangular shape of grid cells#1079
alex-huth merged 2 commits intoNOAA-GFDL:dev/gfdlfrom
OlgaSergienko:metrics

Conversation

@OlgaSergienko
Copy link
Copy Markdown

@OlgaSergienko OlgaSergienko commented Apr 1, 2026

This PR corrects an implementation of a finite-element solver for ice-sheet/shelf velocity. It computes a Jacobian weight J_q in shape functions for quadrature points and applies it to integrals over cells. This correction is significant when the shape of the cell is not rectangular. The same corrections are applied for subgrid parameterizations.

Additionally, it fixes missing deallocations of several arrays.

Changes made:

  • nitialize_ice_shelf_dyn: allocates a new variable Jac
  • bilinear_shape_fn_grid: computes a Jacobian determinant (Jac) at each quadrature point
  • CG_action: uses jac_wt= Jac * IareaT for computing the ice deformation and basal traction terms
  • matrix_diagonal: uses jac_wt= Jac * IareaT for computing diagonal terms
  • ice_shelf_dyn_end: deallocates Jac, CS%sx_shelf, CS%sy_shelf, CS%u_flux_bndry_val, CS%v_flux_bndry_val,CS%calv_mask, CS%Phi, CS%Phisub, CS%PhiC.

Copilot (model Claude Sonet 4.6) was used for this PR.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 8, 2026

Codecov Report

❌ Patch coverage is 0% with 41 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.74%. Comparing base (6074eb3) to head (bf5c582).
⚠️ Report is 13 commits behind head on dev/gfdl.

Files with missing lines Patch % Lines
src/ice_shelf/MOM_ice_shelf_dynamics.F90 0.00% 41 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           dev/gfdl    #1079      +/-   ##
============================================
- Coverage     38.75%   38.74%   -0.01%     
============================================
  Files           274      274              
  Lines         92262    92279      +17     
  Branches      17823    17829       +6     
============================================
- Hits          35753    35751       -2     
- Misses        49814    49832      +18     
- Partials       6695     6696       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown

@alex-huth alex-huth left a comment

Choose a reason for hiding this comment

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

Looks good, other than the changes I have requested regarding where the weight is applied for the viscosity Newton corrections in the preconditioner

Comment thread src/ice_shelf/MOM_ice_shelf_dynamics.F90 Outdated
Comment thread src/ice_shelf/MOM_ice_shelf_dynamics.F90 Outdated
@OlgaSergienko
Copy link
Copy Markdown
Author

Thank you for catching these error, @alex-huth. I agree that jac_wt was applied for the viscosity Newton correction in the wrong place.

This PR corrects an implementation of a finite-element solver for ice-sheet/shelf velocity. It computes a Jacobian weight J_q in shape functions for quadrature points and applies it to integrals over cells. This correction is significant when the shape of the cell is not rectangular. The same corrections are applied for subgrid parameterizations.

Additionally, it fixes missing deallocations of several arrays.

Changes made:
 - nitialize_ice_shelf_dyn: allocates a new variable `Jac`
 - bilinear_shape_fn_grid: computes a Jacobian determinant (`Jac`) at each quadrature point
 - CG_action: uses `jac_wt= Jac * IareaT` for computing the ice deformation and basal traction terms
 - matrix_diagonal: uses `jac_wt= Jac * IareaT` for computing diagonal terms
 - CG_action_subgrid_basal: computes `jac_sub_wt` using new arguments `dxCv_S`, `dxCv_N`,`dyCv_E` and `dyCv_W` that determine cell-edge spacing
 - ice_shelf_dyn_end: deallocates `Jac`, `CS%sx_shelf`, `CS%sy_shelf`, `CS%u_flux_bndry_val`, `CS%v_flux_bndry_val`,`CS%calv_mask`, `CS%Phi`, `CS%Phisub`, `CS%PhiC`.

Copilot (model Claude Sonet 4.6) assisted with this PR.
@OlgaSergienko
Copy link
Copy Markdown
Author

The requested changes have been implemented.

@OlgaSergienko OlgaSergienko requested a review from alex-huth April 20, 2026 18:49
Copy link
Copy Markdown

@alex-huth alex-huth left a comment

Choose a reason for hiding this comment

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

With these changes, the PR looks correct

@alex-huth alex-huth merged commit 57feb58 into NOAA-GFDL:dev/gfdl Apr 20, 2026
52 checks passed
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.

2 participants