DVGeoMulti test fix

[anilyil]

Purpose

The inverse distance warping math in DVGeoMulti uses a small number "eps" to prevent some denominators from being zero or getting a wrong sign. Previously, this clipping was not done in a good way, and the eps value was selected to be too small. See the diff in this PR for the relevant code in DVGeoMulti.

In this PR, I changed the clipping to more accurately represent the computational goal, increased eps from 1e-50 to 1e-20, and retrained the tests.

This is done to address the issue discussed in this PR: #195

This tolerance issue is coming up on this test because some test points are defined to be exactly on the intersection curve. This is a worst-case scenario in terms of numerical accuracy and this is what the eps-based clipping was trying to prevent. Real cases with intersection curves that are not straight are very unlikely to get CFD mesh points that are exactly on the curve. As a result, this change should not affect any real life application.

Increasing eps is the main mechanism that fixes this issue for the worst-case test point. However, while I was at it, I also fixed the clipping math so that its more accurate.

Finally, with the eps value of 1e-50, the ARM and X86 architectures were getting very different answers because of the numerical instability in the algorithm. Slight changes in the updated point coordinates were causing large changes in derivatives as expected. The increased eps value fixes these and the updated points are now consistent to around 10 digits instead of 4-5 with the original eps value.

Expected time until merged

1-2 days

Type of change

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (non-backwards-compatible fix or feature)
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no API changes)
• Documentation update
• Maintenance update
• Other (please describe)

Testing

Checklist

• I have run flake8 and black to make sure the Python code adheres to PEP-8 and is consistently formatted
• I have formatted the Fortran code with fprettify or C/C++ code with clang-format as applicable
• I have run unit and regression tests which pass locally with my changes
• I have added new tests that prove my fix is effective or that my feature works
• I have added necessary documentation

[codecov]

Codecov Report

Merging #199 (d83f395) into main (ba53e25) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #199      +/-   ##
==========================================
- Coverage   64.86%   64.86%   -0.01%     
==========================================
  Files          47       47              
  Lines       11945    11944       -1     
==========================================
- Hits         7748     7747       -1     
  Misses       4197     4197              

Impacted Files	Coverage Δ
pygeo/parameterization/DVGeoMulti.py	91.04% <100.00%> (-0.01%)	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[marcomangano]

Wild catch! Let's merge this asap (@eirikurj we need once again you privileges due to the RTD failures) so we can finally close #195

[eirikurj]

Overall looks good. @anilyil are these tests trained on amd64 or arm64? Just want to check, since we do want to favor amd64 architecture. Just let me know and I can push updated values if you want.

[sseraj]

Looks good, but maybe this comment should also be updated slightly now that we use min?

[anilyil]

I have updated the comment. Please check if the new version makes sense. I also moved the definition of eps to the init so that it is defined once to be consistent between the analysis and derivative routines. Previously it was defined in two places which could have caused some issues if someone forgot to change one of them.

[anilyil]

@eirikurj, as we discussed in our call, the updated reference is trained with the public:u22-gcc-ompi-latest image, with x86 platform, running under emulation on my ARM based mac.

This should not cause issues because the same x86 image running on my mac was passing the tests that were failing on the arm image. The changes in this PR removes the discrepancy from the two platforms so they both pass now.