Replace quad_bilinear_interp with inverse distance weighting in quad_utils. #887
Replace quad_bilinear_interp with inverse distance weighting in quad_utils. #887hkershaw-brown wants to merge 12 commits intomainfrom
Conversation
|
build everything action: https://github.com/NCAR/DART/actions/runs/15426138267 caveat GEOS (Helen are you ever going to fix this? no). edit Yes! #918 |
There was a problem hiding this comment.
Pull Request Overview
This pull request replaces the quad_bilinear_interp interpolation method with an inverse distance weighting (IDW) approach to avoid out-of-bounds issues in quadrilateral interpolation. Key changes include the removal of the bilinear interpolation subroutine, addition of a new quad_idw_interp subroutine with error checking on interpolated values, and updates to build scripts to conditionally include the quad_utils_mod file for the "threed_sphere" location.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| models/utilities/quad_utils_mod.f90 | Removed quad_bilinear_interp and added quad_idw_interp implementing IDW interpolation with additional error handling. |
| build_templates/buildfunctions.sh | Updated source file list to conditionally include quad_utils_mod.f90 for the threed_sphere location. |
| build_templates/buildconvfunctions.sh | Similar conditional inclusion of quad_utils_mod.f90 in the source list for threed_sphere. |
Comments suppressed due to low confidence (1)
models/utilities/quad_utils_mod.f90:2149
- The error handling block uses the variables 'string1', 'string2', and 'source' without any visible declarations or initializations. Please ensure these variables are declared and initialized before they are used to report the error.
if(expected_obs < minval(p) .or. expected_obs > maxval(p)) then
…n quad_utils_mod fixes #860 Same fix as #833 #848 that was for cice model_mod only. Removed the rotation code - see #453 - does not fix the out-of-bounds and is also rotating in the oppostite direction. Not used in IDW. removed svn detritus. Using VERTISUNDEF to get horizontal only distances. I am not sure if this will need to be modififed for other location_mods
The converters in the repo all build succesfully without this change, because they all use threed_sphere location_mod. However maybe there are some converters in the wild that do not use threed_sphere.
6e40a5d to
3c309e8
Compare
I think you're talking about the endless svn junk DART/models/utilities/quad_utils_mod.f90 Lines 85 to 89 in 3c309e8 |
|
Model_mods currently using quad_utils: MOM6/model_mod.f90 |
jlaucar
left a comment
There was a problem hiding this comment.
This looks correct to me. It will definitely not bitwise reproduce. It remains unclear how often the result could be out of the range of the corner values due to roundoff. It we run into applications where this occurs, we may have to remove the error failure so that the currently unused bounds enforcement code comes into play.
jlaucar
left a comment
There was a problem hiding this comment.
This code still looks good to me. However, the comment stating that it is 'unclear' whether round-off could lead to the interpolated value being outside of the range of the vertex values is now clear. The modified code I'm using in aether_cube_sphere occasionally has an interpolated value outside or range when all of the vertex values are identical to machine precision. This comment should probably be changed to
"Round-off can lead to result being outside of range of grid points"
The comment further down should also be changed since we are issuing a MSG rather than an ERR. The part that says "this will not happen with current error check" should be deleted.
that value. Also cleaned up the comments on what happens if the interpolated value is outside the range of the point values.
|
see also #894 |
|
@hkershaw-brown todo quad_util_mod still has the svn relics so the error message is not helpful: |
|
see also #978 |
Make the error message about update_reg_list slightly more helpful #887 (comment)
…r than 0 note E_ALLMSG does not get written to the log file, only std out.
|
also check pole for IDW in addition to #978 |
|
Concern at pole is for wind components which are not well-defined there.
Scalar quantities should work okay.
…On Fri, Feb 20, 2026 at 12:19 PM Helen Kershaw ***@***.***> wrote:
*hkershaw-brown* left a comment (NCAR/DART#887)
<#887 (comment)>
also check pole for IDW in addition to #978
<#978>
—
Reply to this email directly, view it on GitHub
<#887 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ANDHUIQAJDNR4H67JHF2MBT4M5M3JAVCNFSM6AAAAAB6QW3VDGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTSMZWGY2TIMBSG4>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
|
@jlaucar current/wind components on a warped grid. u, v is relative to the grid, or relative to Earth east, north? |
|
It depends on the model as to what is being done. Our fundamental problem
is that DART (and many models) do represent it as east/west and north/south
wind components. Same for some of our observational sources. At the poles,
the east/west component is zero. All winds are north/south only.
…On Fri, Feb 20, 2026 at 12:55 PM Helen Kershaw ***@***.***> wrote:
*hkershaw-brown* left a comment (NCAR/DART#887)
<#887 (comment)>
@jlaucar <https://github.com/jlaucar> current/wind components on a warped
grid. u, v is relative to the grid, or relative to Earth east, north?
Because if it is u,v relative to the grid then you're interpolating say u,
this is not the same direction u at each grid point.
—
Reply to this email directly, view it on GitHub
<#887 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ANDHUITN4YCFC4RCJURO5RT4M5RDVAVCNFSM6AAAAAB6QW3VDGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTSMZWHAZDOMJZHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Description:
The current quad_utils_mod interpolation method (bilinear) can give out-of-bounds values for some quads.
This problem was originally reported by Nuo Chen in CICE #833 when using the QCEFF.
The CICE bilinear interpolation was replaced by inverse distance weighting in #848. This is the same code as quad utils. Note, replacing the CICE interpolation with quad utils is not done in this pull request.
There is a notebook to call dart from python in https://github.com/hkershaw-brown/DART-probe/blob/main/ice-interp.ipynb if you want to reproduce the interpolation plots (see README for dart library build).
Fixes issue
Fixes #860
Fixes #453 since no rotation needed in the IDW. See #833 for plots of rotations
Types of changes
Documentation changes needed?
There is no documentation for quad utils at the moment.
Tests
This is a draft at the moment.
Questions to answer:
bitwise differences for which models - who cares, it is a bug fix vs. loads of people care.
Checklist for merging
Checklist for release
Testing Datasets