Implement area calculation for footprint of 3D objects #378

w-k-jones · 2023-12-04T02:36:07Z

Resolves #333

Changes calculate_area to use bulk statistics for area calculation, and enables area calculation for both 2D and 3D tracked objects. Area is now not calculated with a time dimension when using lat/lon coords, reducing memory usage. A new collapse_dim keyword has been added to get_statistics_from_mask, allowing one or more dimensions of the input labels to be collapsed. This, for example, allows calculation of the 2D area of the footprint of a 3D tracked object.

I have also reorganised analysis into a package, with a separate file for spatial functions. Functions other than calculate_area are identical except for updates to imports

…s of labels to be collapsed

…r more dims of labels to be collapsed

codecov · 2023-12-04T02:38:44Z

Codecov Report

Attention: Patch coverage is 52.94118% with 168 lines in your changes are missing coverage. Please review.

Project coverage is 60.44%. Comparing base (df86e7f) to head (a5f126e).
Report is 9 commits behind head on RC_v1.5.x.

Files	Patch %	Lines
tobac/analysis/cell_analysis.py	12.19%	144 Missing ⚠️
tobac/analysis/feature_analysis.py	22.58%	24 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##           RC_v1.5.x     #378      +/-   ##
=============================================
+ Coverage      57.00%   60.44%   +3.44%     
=============================================
  Files             20       23       +3     
  Lines           3454     3522      +68     
=============================================
+ Hits            1969     2129     +160     
+ Misses          1485     1393      -92

Flag	Coverage Δ
unittests	`60.44% <52.94%> (+3.44%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

… when trying to collapse all axes

…into 3d_area_calculation

…ersion from idealised case notebook

… output

fsenf · 2024-03-04T11:45:45Z

@w-k-jones : Unfortunately, the notebook is still failing to run through after your fix :-(

w-k-jones · 2024-03-04T13:35:06Z

I haven’t addressed the issue with the notebook yet, still deciding what action to take. Pinning the xarray requirements is the easiest but I generally try to avoid doing that

fsenf · 2024-03-06T07:15:12Z

after #411 is done, this PR could be the next one. @w-k-jones : Any idea for a quickfix for the notebook bug?

w-k-jones · 2024-03-12T15:15:52Z

after #411 is done, this PR could be the next one. @w-k-jones : Any idea for a quickfix for the notebook bug?

The easiest fix will be to ensure the correct xarray version, although I'm slightly against doing that

…er masked arrays

w-k-jones · 2024-03-13T21:14:46Z

I have added a fix to the issues with iris->xarray conversion (see convert_cube_to_dataarray in utils.decorators) which handles the situation in which a masked, integer cube would cause an error when converting to xarray. The notebooks now seem to all run correctly, even with older versions of xarray. I have also included updates from RC_v1.5.x and the fixes in #416 , so will wait for that to be merged before this.

@fsenf / @fziegner would you be able to review?

…odules

fsenf · 2024-03-14T07:43:30Z

Yes, great job, @w-k-jones ! CI looks good! We will allocate more time for review tomorrow.

fsenf

Dear Will,

really great work! Esp. the expansion of testing capabilities (~1000 new lines) is exceptional!

I marked a small inconsistency, but I am also happy if you just proceed and merge you devs back to the release candidate.

Cheers, Fabian.

fsenf · 2024-03-15T08:40:27Z

tobac/analysis/spatial.py

@@ -413,7 +413,7 @@ def calculate_area(features, mask, method_area=None, vertical_coord=None):
        )
    elif method_area == "latlon":
        if (mask_slice.coord("latitude").ndim == 1) and (
-            mask_slice.coord("latitude").ndim == 1
+            mask_slice.coord("longitude").ndim == 1


fsenf · 2024-03-15T08:56:35Z

tobac/tests/test_analysis_spatial.py

+    test_labels = xr.DataArray(
+        test_labels.values,
+        dims=(
+            "time",
+            "latitude",
+            "x_dim",
+        ),
+        coords={
+            "time": [datetime(2000, 1, 1), datetime(2000, 1, 1, 1)],
+            "latitude": xr.DataArray(
+                np.arange(5), dims="latitude", attrs={"units": "degrees"}
+            ),
+            "longitude": xr.DataArray(
+                np.stack([np.arange(5)] * 5),
+                dims=("x_dim", "latitude"),
+                attrs={"units": "degrees"},
+            ),
+        },
+    )


This looks inconsistent. I am not sure why you replace "longitude" with "x_dim", but it should be done for "latitude" in a similar way to be consistent.

In this case it's meant to be inconsistent, as I was trying to test the error condition for when a dataset has 1D latitude but 2D longitude (i.e. inconsistent dimensionality on lat/lon). I will update the dimension naming to make it a bit clearer however

fsenf · 2024-03-15T09:04:46Z

tobac/utils/decorators.py

+def convert_cube_to_dataarray(cube):
+    """
+    Convert an iris cube to an xarray dataarray, averting error for integer dtype cubes in xarray<v2023.06
+
+    Parameters
+    ----------
+    cube : iris.cube.Cube
+        Iris data cube
+
+    Returns
+    -------
+    dataarray : xr.DataArray
+        dataarray converted from cube. If the cube's core data is a masked array and has integer dtype,
+        the returned datarray will have a numpy array with masked values filled with the minimum value for
+        that integer dtype. Otherwise the data will be identical to that produced using xr.DataArray.from_iris
+    """
+    if isinstance(cube.core_data(), ma.core.MaskedArray) and np.issubdtype(
+        cube.core_data().dtype, np.integer
+    ):
+        return xr.DataArray.from_iris(
+            cube.copy(cube.core_data().filled(np.iinfo(cube.core_data().dtype).min))
+        )
+    return xr.DataArray.from_iris(cube)


great hack!

w-k-jones · 2024-03-15T10:08:39Z

Thanks @fsenf , I will update the case you highlighted to make my intentions clearer. Will wait until after #416 to merge

freemansw1 · 2024-03-15T17:54:22Z

Thanks @fsenf , I will update the case you highlighted to make my intentions clearer. Will wait until after #416 to merge

I've just merged #416

w-k-jones added 11 commits December 3, 2023 21:13

Reorganise imports

cd3dfaa

Reorganise imports

8a13960

Restructure analysis module

f2086c8

Move distance, velocity and area functions to analysis.spatial

a0c2ed9

Move distance, velocity and area functions to analysis.spatial

8f3b6e9

Rename init file in analysis to fix import issues

b2f92ca

Update setup.py to automatically find package names

fb3731a

Reorganise imports in setup.py

e353f93

Add collapse_axis keyword to get_statistics, allowing one or more axe…

00c1c38

…s of labels to be collapsed

Add collapse_dims keyword to get_statistics_from_mask, allowing one o…

029eccd

…r more dims of labels to be collapsed

Update calculate_area to use bulk statistics

100f2a6

w-k-jones self-assigned this Dec 4, 2023

w-k-jones added this to the Version 1.5.3 milestone Dec 4, 2023

w-k-jones linked an issue Dec 4, 2023 that may be closed by this pull request

bug in calculate_area #333

Closed

w-k-jones added bug Code that is failing or producing the wrong result enhancement Addition of new features, or improved functionality of existing features labels Dec 4, 2023

Change order of input checks in get_statistics to raise correct error…

7d16627

… when trying to collapse all axes

w-k-jones requested review from freemansw1, fsenf, JuliaKukulies and harrietgilmour December 4, 2023 02:47

w-k-jones added 6 commits December 4, 2023 18:12

Reorganise imports and remove unused

1f56e2d

Remove broadcasting warning

361a1f3

Add test for calculate_area with 1D lat/lon coords

4a8f049

Add tests for area calculation using 1D and 2D lat/lon coords

68f2f54

Merge branch 'RC_v1.5.x' of https://github.com/climate-processes/tobac …

35313bf

…into 3d_area_calculation

Update pylint workflow to trigger on pull_request_target

929483f

w-k-jones mentioned this pull request Dec 4, 2023

Add Pylint to Workflow #373

Merged

11 tasks

w-k-jones force-pushed the 3d_area_calculation branch from 55e54f2 to 929483f Compare December 4, 2023 23:08

w-k-jones added 3 commits March 2, 2024 14:00

Add xarray_to_iris decorator to calculate_areas, remove explicit conv…

26334c9

…ersion from idealised case notebook

Rename mask to segment_labels and correct description of segmentation…

66d664c

… output

Reformatting

b77d21b

fix issues with decorators

3bc47ca

freemansw1 mentioned this pull request Mar 13, 2024

Xarray/Iris incompatibilities around integer arrays #418

Closed

w-k-jones added 4 commits March 13, 2024 20:52

Fix merge conflicts from RC_v1.5.x

99b1bf8

Merge changes from freemansw1/fix_1.5.x_issues

a49a692

Add new conversion function for iris cubes to handle error with integ…

01c324e

…er masked arrays

Formatting

7f31002

Update API reference docs for analysis package and add all utils subm…

7d393b4

…odules

w-k-jones added 8 commits March 14, 2024 08:38

Add tests for decorators

9a6b560

Add tests for calculate_distance and calculate_velocity_individual

ba68e2a

Add test for calculate_velocity

db09d7e

Add tests for calculate_nearestneighbordistance

242d0de

Add tests for lat/lon area and exception handling in calculate_area

313f024

Fix coordinate/dim naming clash

683aff0

Fix test catching wrong exception

388d123

Fix wrong coordinate name in calculate_area

6563569

fsenf approved these changes Mar 15, 2024

View reviewed changes

Clarify tests for invalid lat/lon and add test for 3D lat/lon

a5f126e

w-k-jones merged commit 3a8ffba into tobac-project:RC_v1.5.x Mar 15, 2024
20 of 21 checks passed

w-k-jones mentioned this pull request Mar 15, 2024

bug in calculate_area #333

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement area calculation for footprint of 3D objects #378

Implement area calculation for footprint of 3D objects #378

w-k-jones commented Dec 4, 2023 •

edited

Loading

codecov bot commented Dec 4, 2023 •

edited

Loading

fsenf commented Mar 4, 2024

w-k-jones commented Mar 4, 2024

fsenf commented Mar 6, 2024

w-k-jones commented Mar 12, 2024

w-k-jones commented Mar 13, 2024 •

edited

Loading

fsenf commented Mar 14, 2024

fsenf left a comment

fsenf Mar 15, 2024

fsenf Mar 15, 2024

w-k-jones Mar 15, 2024

fsenf Mar 15, 2024

w-k-jones commented Mar 15, 2024

freemansw1 commented Mar 15, 2024

Implement area calculation for footprint of 3D objects #378

Implement area calculation for footprint of 3D objects #378

Conversation

w-k-jones commented Dec 4, 2023 • edited Loading

codecov bot commented Dec 4, 2023 • edited Loading

Codecov Report

fsenf commented Mar 4, 2024

w-k-jones commented Mar 4, 2024

fsenf commented Mar 6, 2024

w-k-jones commented Mar 12, 2024

w-k-jones commented Mar 13, 2024 • edited Loading

fsenf commented Mar 14, 2024

fsenf left a comment

Choose a reason for hiding this comment

fsenf Mar 15, 2024

Choose a reason for hiding this comment

fsenf Mar 15, 2024

Choose a reason for hiding this comment

w-k-jones Mar 15, 2024

Choose a reason for hiding this comment

fsenf Mar 15, 2024

Choose a reason for hiding this comment

w-k-jones commented Mar 15, 2024

freemansw1 commented Mar 15, 2024

w-k-jones commented Dec 4, 2023 •

edited

Loading

codecov bot commented Dec 4, 2023 •

edited

Loading

w-k-jones commented Mar 13, 2024 •

edited

Loading