Update IS_IN_UNGRID to handle if grid lon defintions are mismatched #1325
Conversation
|
@JessicaMeixner-NOAA thank you for tracking this down. Sorry for the delayed reply, I'm going to get the tests going now. |
There was a problem hiding this comment.
@JessicaMeixner-NOAA my testing gives the same output you got regarding b4b's.
**********************************************************************
********************* non-identical cases ****************************
**********************************************************************
mww3_test_03/./work_PR1_MPI_e (1 files differ)
mww3_test_03/./work_PR3_UQ_MPI_e_c (1 files differ)
mww3_test_03/./work_PR3_UNO_MPI_e (1 files differ)
mww3_test_03/./work_PR2_UQ_MPI_e (1 files differ)
mww3_test_03/./work_PR2_UNO_MPI_e (1 files differ)
mww3_test_03/./work_PR2_UNO_MPI_d2 (14 files differ)
mww3_test_03/./work_PR1_MPI_d2 (15 files differ)
mww3_test_03/./work_PR3_UNO_MPI_d2_c (8 files differ)
mww3_test_03/./work_PR3_UQ_MPI_d2_c (16 files differ)
mww3_test_03/./work_PR3_UNO_MPI_d2 (15 files differ)
mww3_test_03/./work_PR2_UQ_MPI_d2 (14 files differ)
mww3_test_03/./work_PR3_UQ_MPI_e (1 files differ)
mww3_test_03/./work_PR3_UNO_MPI_e_c (1 files differ)
mww3_test_03/./work_PR3_UQ_MPI_d2 (16 files differ)
mww3_test_09/./work_MPI_ASCII (0 files differ)
ww3_tp2.10/./work_MPI_OMPH (6 files differ)
ww3_tp2.16/./work_MPI_OMPH (4 files differ)
ww3_tp2.21/./work_ma (3 files differ)
ww3_tp2.21/./work_mb (3 files differ)
ww3_tp2.6/./work_ST4_ASCII (0 files differ)
ww3_ufs1.1/./work_unstr_b (4 files differ)
ww3_ufs1.1/./work_unstr_a (4 files differ)
ww3_ufs1.1/./work_unstr_c (4 files differ)
ww3_ufs1.3/./work_a (3 files differ)
**********************************************************************
************************ identical cases *****************************
**********************************************************************
I'm okay approving this, I think first it would be good to know what plan you had in mind for addressing the unexpected non-b4b's?
ww3_tp2.21/./work_ma (3 files differ)
ww3_tp2.21/./work_mb (3 files differ)
|
My preference would be to create an issue to investigate what is going on in that regression test - which has issues regardless of this PR, the interpolation is not performing as I think we would expect. I can create that issue if you agree on this path forward or please suggest an alternative. |
Since it's not related to this PR, I agree that creating in issue to look into it is a good path forward. |
|
Issue made: #1332 |
Looks great! Thank you @JessicaMeixner-NOAA, I'll wrap up the review. |
There was a problem hiding this comment.
Code Review
Pass
Testing
Pass
See #1325 (review) for matrixCompSummary non-b4b's.
- matrixCompSummary.txt
- matrixCompFull.txt
- Note matrixDiff.txt was too large to post in this case.
Approved.
|
Thank you @JessicaMeixner-NOAA for fixing this mismatch, and posting the issue to follow up on the non-b4b's. |
Pull Request Summary
Update IS_IN_UNGRID to handle if grid lon defintions are mismatched
Description
This PR adds a modified longitude point for the point that you are trying to find in a triangle element. If for example this point is -180 but the grid is defined from 0 to 360 this will address this mismatch and the point will be found in an element.
If there are regtests where the requested point output does not match the grid definition (ufs1.1) then there will be answer changes. This followed the work of the "fix periodicity" routine.
Issue(s) addressed
Fixes #1273
Commit Message
Update IS_IN_UNGRID to handle if grid lon defintions are mismatched
Check list
Testing
We expect that unstructured grid regtests with points defined outside of it's domain definition, such as ufs1.1 will have answer changes.
Not originally expected non-b4b:
Looking into this test case closer, it looks like we're actually finding more points for the interpolation. This test might actually need further investigation (unrelated to this PR) as the interpolated output doesn't seem quite right. However it's the same before and after this PR, but the weights are slightly different.
The ufs1.1 tests now have more output points - indicating this fix is working.