Skip to content

Quad Utilities: Enhanced Stability and New Documentation #1089

Open
mgharamti wants to merge 4 commits intoNCAR:mainfrom
mgharamti:improve_quad_utils
Open

Quad Utilities: Enhanced Stability and New Documentation #1089
mgharamti wants to merge 4 commits intoNCAR:mainfrom
mgharamti:improve_quad_utils

Conversation

@mgharamti
Copy link
Copy Markdown
Contributor

Description:

This PR improves the numerical robustness of quadrilateral interpolation in quad_utils_mod and introduces new module documentation.

For the interpolation code, Cramer's rule (for solving the 3×3 system inquad_bilinear_interp) is now replaced with Gaussian Elimination with partial Pivoting (GEP). The new solver improves numerical stability, particularly for highly distorted or nearly degenerate quadrilateral cells. The computational cost remains constant (3×3 system) and negligible relative to the overall interpolation workflow.

I also added a new rst documentation page for quad_utils_mod. The documentation provides an overview of the interpolation procedure with detailed description of the supported grids, interpolation methods, and workflow.

Fixes issue

Fixes issue #1081

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Documentation changes needed?

  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.

Tests

I ran the developer tests for the quad_utils and also tested the new changes on small grids using different distorted quads. I got identical results between previous and refactored implementations for well-conditioned grids. For pathological quad configurations, the new solver demonstrates improved accuracy (orders of magnitude reduction in error).

Checklist for merging

  • Updated changelog entry
  • Documentation updated
  • Update conf.py

Checklist for release

  • Merge into main
  • Create release from the main branch with appropriate tag
  • Delete feature-branch

Testing Datasets

  • Dataset needed for testing available upon request
  • Dataset download instructions included
  • No dataset needed

Replaced Cramer's rule with Gaussian elimination with partial
pivoting. This improves numerical stability especially for
severely distored quadrilaterals.
@mgharamti mgharamti added Documentation Improvements or additions to documentation quad_utils interpolation labels Mar 29, 2026
Comment thread models/utilities/quad_utils_mod.f90 Outdated
'failed. It could be a singularity or a very tiny pivot'
write(string2, '(2(A, F10.4))') 'Obs location -> lon: ', lon, ', lat: ', lat
write(string3, '(2(A, 4F10.4))') 'Quad corners -> x: ', x_corners, ', y: ', y_corners
call error_handler(E_MSG, 'quad_bilinear_interp', string1, &
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This message will only print on task 0, so if other tasks have failures you will not see the message. To have the message print on any task:
E_ALLMSG

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

nice catch! I'll update this

! compute deter of m
!d = deter3(m)

! Solve the matrix for b, c and d
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@mgharamti Does this method need the do_rotate namelist option?

I ask because the code I think is doing a clockwise rotation (vs counterclockwise as the comment stays)
#833 (comment)
so if the rotation is not needed for stability with this new method, I think we should remove the rotation code.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Helen, in my tests rotations didn't add anything. I find the new solver to be stable without the need for rotations. Would that apply to the CICE examples is the issue you shared? probably.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The do_rotate true/false did not improve the out-of-bounds for the CICE

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

hmm, ok. I don't know if users actively use this rotation option. For legacy reasons, we could keep it. It's defaulted to .false. anyways.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ok I will put it on the todo list for the future to remove, and we leave it in for this pull request.

@@ -0,0 +1,200 @@
.. _quad_utils_mod:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice docs, I think it would be good to link to the quad_utils_mod from the "required model mod routines" doc

Definitely here:
https://docs.dart.ucar.edu/en/latest/guide/required-model-mod-routines.html#required-model-mod-routines

Maybe here?:
https://docs.dart.ucar.edu/en/latest/guide/advice-for-new-collaborators.html#interpolation

Something like:
"DART provides tools in the quad_utils_mod module for both grid search and interpolation on logically rectangular grids, that is, grids indexed like a regular (i,j) array but potentially curvilinear in physical space. These quad_utils_mod routines can be used within model_interpolate() to avoid having to implement interpolation routines from scratch"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good idea, will do.

Mask Handling
^^^^^^^^^^^^^

An optional mask can be provided for fully irregular grids. If any of the four
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is just a curiosity question, why do only irregular grids except masks?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Helen, good question. To me, I think of the regular grid's interpolation as index-based. We don't need mask to define geometry; the grid itself is valid everywhere. So, masking here is handled outside (e.g., point_on_land, quad_on_land, ...).

On the other hand, interpolation in the irregular grids is geometry-based (physical space) in which we need the 4 corners to fit a surface. If one corner is land (or invalid), this will corrupt the interpolation. In essense, if any of the corners is masked we do not interpolate.

We could add masks for regular grid, but it adds overhead. It's essential in the irregular case for correctness.

Added references to the ``quad_utils_mod`` in 2 pages: guide/
required-model-mod-routines and advice-for-new-collaborators

Also, changed message reporting when quad_interp fails to be
done across all tasks.
@hkershaw-brown
Copy link
Copy Markdown
Member

hkershaw-brown commented Apr 15, 2026

Question on the tests:

also tested the new changes on small grids using different distorted quads. I got identical results between previous and refactored implementations for well-conditioned grids.

Are these test refering to test_quad_reg_interp (well-conditioned grids) and test_quad_irreg_interp ( different distorted quads) or do you have other tests?

I ran the developer tests for the quad_utils

I'm not sure what the test is in the developer tests for quad_utils are testing, there's not a pass fail. So I just compared the .txt output from main and mgharamti:improve_quad_utils, (and plotted the results #1097)

The results are different on the last digit between main and this branch for test_quad_irreg_interp. I do not expect they would be bitwise between the 2 algorithms, because this is the point of the pull request.
Both this branch and main give 'large interp residual' messages.

[hkershaw:work] (improve_quad_utils) > ./test_quad_irreg_interp | grep 'large interp residual' | wc -l
17049

[hkershaw:work] (main) > ./test_quad_irreg_interp | grep 'large interp residual' | wc -l
15953

gfortran --version
GNU Fortran (MacPorts gcc13 13.3.0_2+stdlib_flag) 13.3.0
Copyright (C) 2023 Free Software Foundation, Inc.

@mgharamti
Copy link
Copy Markdown
Contributor Author

Helen, I built my own distorted quad tests. I also ran the the ones in the developer tests but I didn't find them very informative.

@hkershaw-brown
Copy link
Copy Markdown
Member

Helen, I built my own distorted quad tests. I also ran the the ones in the developer tests but I didn't find them very informative.

cool, I figured you had something cooler from the plots you showed at standup.

Copy link
Copy Markdown
Member

@hkershaw-brown hkershaw-brown left a comment

Choose a reason for hiding this comment

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

Approved!

@hkershaw-brown hkershaw-brown added the release! bundle with next release label Apr 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Documentation Improvements or additions to documentation interpolation quad_utils release! bundle with next release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants