Feature/relative matching distance

[chahank]

Changes proposed in this PR:

• This PR contains the changes from Remove implicit use of CRS in code base #1020
• In addition, it adds a relative matching threshold based on the equivalent grid spacing of the centroids.

This PR fixes #1079.

PR Author Checklist

• Read the Contribution Guide
• Correct target branch selected (if unsure, select develop)
• Descriptive pull request title added
• Source branch up-to-date with target branch
• Documentation updated
• Tests updated
• Tests passing
• No new linter issues
• Changelog updated

PR Reviewer Checklist

• Read the Contribution Guide
• CLIMADA Reviewer Checklist passed
• Tests passing
• No new linter issues

[luseverin]

Ok so I added check_if_geo_coords in lon_normalize and updated the couple tests that could be easily updated (essentially, removed test data that was not consistent with our definition of geographic coordinates). A few other things happened in addition:

• test_set_one_file_pass was failing because an empty lon array was inputed to lon_normalizewithin the call to tropCyclone.from_tracks(tc_track, centroids=CENTR_TEST_BRB) . I fixed it by adding an explicit handling of empty arrays in is_geo_coords, where empty arrays are considered as valid geographic coordinates. I do not know if this is a desirable feature, and I also do not know if the empty lon array in tropCyclone.from_tracks(tc_track, centroids=CENTR_TEST_BRB) is a bug or not. It seems that the idx_centr_filter variable becomes an empty array at some point, leading to an empty centroids array, but I did not investigate this further..
• In test_ibtracs_with_basin, the track downloaded with tc_track = tc.TCTracks.from_ibtracs_netcdf(year_range=(2002, 2003), basin="WP", genesis_basin="EP") has longitudinal value that have extent > 360 (lon_min=-179, lon_max=182.6). I increased the tolerance in is_geo_coords for the longitudinal extent from 361 to 541 as a fix. It seems to be working for now and I hope this should work for all tracks that one can get from IBtracks but this is only a guess..
• A couple tests from the LitPop module are now failing: test_switzerland300_pass and test_Liechtenstein_15_lit_pass. It seems that some parameters of the Affine rasterio transform have been changed, e.g. ent.meta["transform"][4] == 0.08333333000000209 instead of -0.08333333333333333, and ent.meta["transform"][5] == 45.833333335 instead of 47.75. @chahank any idea where this might come from?

[chahank]

Ok so I added check_if_geo_coords in lon_normalize and updated the couple tests that could be easily updated (essentially, removed test data that was not consistent with our definition of geographic coordinates). A few other things happened in addition:

* `test_set_one_file_pass` was failing because an empty lon array was inputed to `lon_normalize`within the call to `tropCyclone.from_tracks(tc_track, centroids=CENTR_TEST_BRB) `. I fixed it by adding an explicit handling of empty arrays in `is_geo_coords`, where empty arrays are considered as valid geographic coordinates. 

Excellent!

I do not know if this is a desirable feature, and I also do not know if the empty lon array in tropCyclone.from_tracks(tc_track, centroids=CENTR_TEST_BRB) is a bug or not. It seems that the idx_centr_filter variable becomes an empty array at some point, leading to an empty centroids array, but I did not investigate this further..

* In `test_ibtracs_with_basin`, the track downloaded with` tc_track = tc.TCTracks.from_ibtracs_netcdf(year_range=(2002, 2003), basin="WP", genesis_basin="EP")` has longitudinal value that have extent > 360 (lon_min=-179, lon_max=182.6). I increased the tolerance in `is_geo_coords` for the longitudinal extent from 361 to 541 as a fix. It seems to be working for now and I hope this should work for all tracks that one can get from IBtracks but this is only a guess..

Great. Weird that IBTrACS basically has invalid coordinates, but it works ;).
Could once try to load all IBTrACS?

* A couple tests from the LitPop module are now failing:  `test_switzerland300_pass` and `test_Liechtenstein_15_lit_pass`. It seems that some parameters of the Affine rasterio transform have been changed, e.g. `ent.meta["transform"][4] == 0.08333333000000209` instead of `-0.08333333333333333`, and `ent.meta["transform"][5] == 45.833333335` instead of `47.75`. @chahank any idea where this might come from?

Was this changed in this PR? Then it might have been a mistake on my side. Please feel free to revert these changes.

[chahank]

@luseverin : several test in petals are still failing. So we need a PR to fix the tests in petals too (and check that nothing fundamental breaks).

[luseverin]

I checked the failing tests in Petals; they were related to lon arrays containing nans or being fully populated with nans being considered as invalid geo coordinates by is_geo_coords. I fixed it by adding a proper handling of nans using np.nanmin and np.nanmax and treating full-nans arrays as valid geo coordinates (same as for empty arrays). Now it seems that everything is passing (besides compatibility tests in petals but this does not seem related to this PR). In addition, tests in core that were previously failing for unknown reasons (test_switzerland300_pass and test_Liechtenstein_15_lit_pass) are now passing (I don't know exactly what changed). @chahank any thoughts? What else do we still need to do?

[luseverin]

Could once try to load all IBTrACS?

I did that and everything worked, by the way.

[chahank]

I checked the failing tests in Petals; they were related to lon arrays containing nans or being fully populated with nans being considered as invalid geo coordinates by is_geo_coords. I fixed it by adding a proper handling of nans using np.nanmin and np.nanmax and treating full-nans arrays as valid geo coordinates (same as for empty arrays). Now it seems that everything is passing (besides compatibility tests in petals but this does not seem related to this PR). In addition, tests in core that were previously failing for unknown reasons (test_switzerland300_pass and test_Liechtenstein_15_lit_pass) are now passing (I don't know exactly what changed). @chahank any thoughts? What else do we still need to do?

Excellent! Just to be clear, there was no need to modify anything in petals, just update the valid geo coordinates method to ignore nans?

[luseverin]

Excellent! Just to be clear, there was no need to modify anything in petals, just update the valid geo coordinates method to ignore nans?

Yes, the tests that were failing in petals could be fixed by adding the handling of nans in is_geo_coords in core. I did not change anything in petals.

[chahank]

Oh, one last thing we should do, since this affects the core climada computation, is to check whether the tutorials still work.

I did not find any issues in the tutorials.

@luseverin : should we update the impact calc tutorial and explicitly set the threshold? Or is it fine as is?

[luseverin]

Oh, one last thing we should do, since this affects the core climada computation, is to check whether the tutorials still work.

I did not find any issues in the tutorials.

@luseverin : should we update the impact calc tutorial and explicitly set the threshold? Or is it fine as is?

We need to coordinate this with @sarah-hlsn as she currently updating this tutorial. It currently seems that there is no explicit assignement of centroids in the impact_calc tutorial. As @sarah-hlsn and I thought it might be good to have that, she will add a short example or note on that, also describing how one can change the distance threshold for centroid assignement. What do you think?

[chahank]

Oh, one last thing we should do, since this affects the core climada computation, is to check whether the tutorials still work.

I did not find any issues in the tutorials.
@luseverin : should we update the impact calc tutorial and explicitly set the threshold? Or is it fine as is?

We need to coordinate this with @sarah-hlsn as she currently updating this tutorial. It currently seems that there is no explicit assignement of centroids in the impact_calc tutorial. As @sarah-hlsn and I thought it might be good to have that, she will add a short example or note on that, also describing how one can change the distance threshold for centroid assignement. What do you think?

Perfect! Then let's finish this PR and merge so that @sarah-hlsn can update the tutorial.

[luseverin]

@chahank Please let me know what we still need to do before we can merge this PR.

[chahank]

@chahank Please let me know what we still need to do before we can merge this PR.

I think we are done, so I merged! Thanks for the great work!