Spatial interp method

[jsmariegaard]

Allow user to select spatial interp method when matching.

Not sure what the new argument should be called 🤔. My first idea was spatial_interp_method, but it's long and when the choice is "contained" it doesn't seem like a good name. Maybe spatial_method is better? Or spatial_matching?

Choosing good default is not easy. 😬

Before this PR the defaults were:

• Dfsu: nearest point for track; contained for point
• Grid: linear for track; nearest for point.

We need to clean this up before the release!

Proposed new spatial interpolation defaults:

• Dfsu: inverse_distance for both point and track (both will be linear in time).
• Grid: linear for both point and track.

Some problems/considerations:

• Dfsu: option "contained" is only supported for point, not track
• Dfsu: in MIKE IO the default n_nearest is 3 for points, 5 for tracks (3 seems to little for quad elements), in this PR the default is so far 5 points, but this is then not the same as you would get by default in MIKE IO which could be confusing
• Grid: on track extraction the same interp method is used in both space and time. This is different than all other extractions, but technically hard to get around.
• If user is matching several models at the same time and both Dfsu and Grid are present, then it's hard to chose anything else than spatial_interp_method=None, as there are no good options that hold for both Dfsu and Grid. Should we through an error/warning if user tries to set spatial_interp_method in this case?

[jsmariegaard]

Changing the Dfsu default for track extraction to "inverse_distance" makes these tests fail:

FAILED tests/test_multimodelcompare.py::test_mm_skill_obs - assert 0.07769535073136861 ± 7.8e-08 == 0.081431053
FAILED tests/test_multimodelcompare.py::test_mm_mean_skill_obs - assert 0.11124922287184795 ± 1.1e-07 == 0.11113215
FAILED tests/test_multimodelcompare.py::test_mm_mean_skill_area_polygon - assert 0.33783800895043575 ± 3.4e-07 == 0.3349027897
FAILED tests/test_multimodelcompare.py::test_mm_mean_skill - assert 0.3093435676524284 == 0.309118939 ± 3.1e-07
FAILED tests/test_multimodelcompare.py::test_mm_mean_skill_weights_list - assert 0.3266280710655306 == 0.3261788143 ± 3.3e-07
FAILED tests/test_multimodelcompare.py::test_mm_mean_skill_weights_str - assert 0.33686945463407164 == 0.3367349 ± 3.4e-07
FAILED tests/test_multimodelcompare.py::test_mm_mean_skill_weights_dict - assert 0.3266280710655306 == 0.3261788143 ± 3.3e-07
FAILED tests/test_multivariable_compare.py::test_mv_mm_mean_skill - assert 0.6392425352313883 ± 6.4e-07 == 0.63344531

[jsmariegaard]

Changing the Dfsu default for point extraction from "contained" (=isel) to "nearest" does not make sense (we should maybe consider if it should even be allowed), and it is not easy to change it to "inverse_distance" as it is not directly supported for Dfsu2DH objects (that we use for .dfsu files), but only for mikeio.Datasets 🙄. So if we wanna change the default to inverse_distanc, we should first find nearest points, then interpolator and then multiply together and create a mikeio.Dataset. Maybe better if this functionality was in MIKE IO...

We would need something like this (incomplete):

elemids = self.data.geometry.find_nearest_elements(
    x, y, n_nearest=n_nearest
)
ds = self.data.read(elements=elemids, items=self.sel_items.all)

interpolant = self.data.geometry.get_2d_interpolant(
                    coords, n_nearest=n_nearest, **kwargs
                )
dai = self.data.geometry.interp2d(self, *interpolant).flatten()

[jsmariegaard]

I have already changed the spatial interp method of GridModelResult point extraction from "nearest" to "linear". It required changing 1 test and 1 notebook. It is less robust but at least consistent with track extraction now which also uses "linear".

[ecomodeller]

Remind me why we are doing our own interpolation instead of using https://docs.scipy.org/doc/scipy/reference/generated/scipy.interpolate.griddata.html for interpolation in unstructured grids.

[jsmariegaard]

Remind me why we are doing our own interpolation instead of using https://docs.scipy.org/doc/scipy/reference/generated/scipy.interpolate.griddata.html for interpolation in unstructured grids.

It can be hard to remember. It's been a long time since I implemented this in MIKE IO. But as far as I remember I struggled to find something efficient enough for multi-timestep situations (you don't to re-calculate the weights for every timestep). But there could probably be something out there which does a much better job!

[jsmariegaard]

I have now implemented the inverse distance weighting for dfsu files too. Changing the default to inverse distance, gives these pytest fails:

==================================================== short test summary info ====================================================
FAILED tests/test_aggregated_skill.py::test_skill_mm_sort_values - AssertionError: assert ['SW_1', 'HKNA'] == ['SW_2', 'c2']
FAILED tests/test_config.py::test_comparison_from_yml - assert 0.2331039633116363 == 0.23287298772 ± 2.3e-07
FAILED tests/test_multimodelcompare.py::test_mm_skill - assert 0.28321342914778314 ± 2.8e-06 == 0.214476
FAILED tests/test_multimodelcompare.py::test_mm_mean_skill_obs - assert 0.1149489254094655 ± 1.1e-07 == 0.11113215
FAILED tests/test_multimodelcompare.py::test_mm_skill_area_bbox - assert 0.3172098772905674 ± 3.2e-07 == 0.293498777
FAILED tests/test_multimodelcompare.py::test_mm_mean_skill_area_polygon - assert 0.33510409783863826 ± 3.4e-07 == 0.3349027897
FAILED tests/test_multimodelcompare.py::test_mm_skill_metrics - assert -0.07751961209876564 ± 7.8e-08 == -0.06659714
FAILED tests/test_multimodelcompare.py::test_mm_mean_skill - assert 0.3423446002875196 == 0.309118939 ± 3.1e-07
FAILED tests/test_multimodelcompare.py::test_mm_mean_skill_weights_list - assert 0.3397577440901333 == 0.3261788143 ± 3.3e-07
FAILED tests/test_multimodelcompare.py::test_mm_mean_skill_weights_str - assert 0.40227215393665355 == 0.3367349 ± 3.4e-07
FAILED tests/test_multimodelcompare.py::test_mm_mean_skill_weights_dict - assert 0.3397577440901333 == 0.3261788143 ± 3.3e-07
FAILED tests/test_multivariable_compare.py::test_mv_skill - assert 0.22792652797730392 ± 2.3e-07 == 0.22359663
FAILED tests/test_multivariable_compare.py::test_mv_mm_skill - assert 1.5610323283542897 ± 1.6e-06 == 1.27617894455
FAILED tests/test_multivariable_compare.py::test_mv_mm_mean_skill - assert 0.6188724432948591 ± 6.2e-07 == 0.63344531
FAILED tests/test_pointcompare.py::test_score - assert 0.19870695987855852 == 0.1986296276629835 ± 2.0e-07
FAILED tests/test_pointcompare.py::test_weighted_score - assert 0.19870695987855852 == 0.1986296276629835 ± 2.0e-07
FAILED tests/test_pointcompare.py::test_mod_aux_carried_over - assert -0.017160136296410562 == -0.0360998 ± 3.6e-08
=============================== 17 failed, 511 passed, 6 skipped, 1 warning in 128.79s (0:02:08) ================================

[jsmariegaard]

@ecomodeller , I will merge this at 10ish if you have no further comments