NOC-MSM / pyBDY

pyBDY: a Python based regional NEMO model configuration toolbox.
GNU General Public License v3.0
7 stars 7 forks source link

issue 127, a fix for the temp and salinity interpolation issues #147

Closed anwiseNOCL closed 4 days ago

anwiseNOCL commented 1 year ago

This should fix the error in the temperature and salinity fields in the near bottom grid cells described in #127.

The issue seems to be that horizontal interpolation was incorrectly handling non-wet points in the source file (assuming they are NaNs rather than zeros). This meant that close to the bed the denominator in mean calculation did not account for the fact that fewer points are used in the interpolation.

Thanks to @swakelin for identifying this.

Testing:

Previous behaviour: The bdy data I generated from GO8 caused AMM7 to become unstable because of the spurious values near the bottom of the water column (I think this effects the velocities as well).

New behaviour: I have regenerated the bdy files and now AMM7 runs are stable.

Closes #127

jdha commented 1 year ago

Just back from leave - so will pick this up shortly. Just a note (more to myself than anything) - I think that the original code used the source mask to turn non-wet points into NaNs ... so this was not an assumption. Perhaps the logic has been broken somewhere between the bitbucket and GitHub versions. @anwiseNOCL did this change the result of the benchmark test?

anwiseNOCL commented 1 year ago

@jdha @JMorado By the way I found that as part of the pre-push process my pybdy conda environment was updated. Scipy gets updated to 1.11.1 and introduces a breaking change. It affects how scipy.spatial.ckdtree handles nans and this cause it to error when do the vertical interpolation I think. After much confusion I reverted back to 1.10.1 and it works again.

JMorado commented 1 year ago

@jdha @JMorado By the way I found that as part of the pre-push process my pybdy conda environment was updated. Scipy gets updated to 1.11.1 and introduces a breaking change. It affects how scipy.spatial.ckdtree handles nans and this cause it to error when do the vertical interpolation I think. After much confusion I reverted back to 1.10.1 and it works again.

Should we specify scipy=1.10.1 on the environment.yml file to avoid this issue for now?