Fixing coordinate issues with max_proj #1291

shanaxel42 · 2019-04-29T20:24:44Z

Fixes #1214

ttung · 2019-04-29T20:25:39Z

Why do we still want this since the MaxProject filter exists?

shanaxel42 · 2019-04-29T20:27:41Z

Why do we still want this since the MaxProject filter exists?

MaxProject just uses Imagestack.max_proj, so it'll have the same issues

codecov-io · 2019-04-29T20:35:59Z

Codecov Report

Merging #1291 into master will increase coverage by 0.43%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1291      +/-   ##
==========================================
+ Coverage   88.84%   89.28%   +0.43%     
==========================================
  Files         146      147       +1     
  Lines        5246     5542     +296     
==========================================
+ Hits         4661     4948     +287     
- Misses        585      594       +9

Impacted Files	Coverage Δ
starfish/core/imagestack/imagestack.py	`94.01% <100%> (+0.1%)`	⬆️
starfish/core/imagestack/parser/numpy/__init__.py	`90.76% <100%> (+7.91%)`	⬆️
...ots/_detect_spots/trackpy_local_max_peak_finder.py	`90.27% <0%> (-1.39%)`	⬇️
starfish/core/pipeline/algorithmbase.py	`96.66% <0%> (-1.07%)`	⬇️
...ore/intensity_table/intensity_table_coordinates.py	`100% <0%> (ø)`	⬆️
starfish/core/image/_filter/call_bases.py	`97.29% <0%> (ø)`
starfish/core/experiment/experiment.py	`82.51% <0%> (+0.31%)`	⬆️
starfish/core/intensity_table/intensity_table.py	`96.92% <0%> (+0.49%)`	⬆️
starfish/core/codebook/codebook.py	`97.26% <0%> (+0.6%)`	⬆️
starfish/core/imagestack/parser/crop.py	`93.1% <0%> (+0.79%)`	⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 30c7680...cbddb91. Read the comment docs.

ttung · 2019-04-29T22:09:48Z

MaxProject just uses Imagestack.max_proj, so it'll have the same issues

LOL ok my bad. :)

ttung

Kind of curious: if you take a max projection across >1 ZPLANES, what is the correct Z coordinate of the result? I think we should have a rational answer for that, and test for it.

ttung · 2019-04-29T22:10:56Z

starfish/core/imagestack/parser/numpy/__init__.py

@@ -142,8 +142,8 @@ def get_tile(self, r: int, ch: int, z: int) -> TileData:
                max_selectors[PHYSICAL_COORDINATE_DIMENSION] = max_selector_value.value

                coordinates[coordinate_type] = (
-                    self.coordinates.loc[min_selectors].item(),


can't you get rid of L139-L142 now?

oh yea good call

shanaxel42 · 2019-04-29T23:29:32Z

Kind of curious: if you take a max projection across >1 ZPLANES, what is the correct Z coordinate of the result? I think we should have a rational answer for that, and test for it.

Every zplane has it's own zc value so would't it just be that value?

neuromusic · 2019-04-30T04:31:07Z

if you take a max projection across >1 ZPLANES, what is the correct Z coordinate of the result?

the mean of the coordinates of the individual zplanes

ttung · 2019-04-30T17:24:32Z

Every zplane has it's own zc value so would't it just be that value?

When you max-project along Z, all the zplanes get flattened into one. So which zplane would the zc be obtained from?

the mean of the coordinates of the individual zplanes

Furthermore, if the zplanes are not evenly spaced, then you would need to average all of the individual z coordinates, rather than just take the first and last and average them. :)

ttung

There needs to be a test to see if we save the correct coordinate when we flatten across >1 planes.

shanaxel42 · 2019-04-30T18:20:01Z

When you max-project along Z, all the zplanes get flattened into one. So which zplane would the zc be obtained from?

But doesn't this happen by just choosing the max value from whatever dim your max_projecting on? And we've already calculated a value for each z_pane. So in this case we'd choose the max z_plane and then the z_coord is whatever we'd previously calculate for that z_plane?

ttung · 2019-04-30T18:47:17Z

For each pixel, you could be sourcing the value from a different z plane. However, we only store a coordinate value per plane, and so when you flatten to one, you have to store only one z coordinate.

kevinyamauchi · 2019-05-13T19:35:15Z

@shanaxel42, what is the status of this PR? I am trying to add other types of projection (e.g., sum across axes) in #1342 and I was hoping to integrate the changes here for transferring physical coordinates.

shanaxel42 · 2019-05-13T21:17:31Z

@shanaxel42, what is the status of this PR? I am trying to add other types of projection (e.g., sum across axes) in #1342 and I was hoping to integrate the changes here for transferring physical coordinates.

Hey! Sorry this one kinda fell off my radar, I'll get it merged today!

kevinyamauchi · 2019-05-13T22:11:25Z

Awesome! Thanks!

ttung · 2019-05-14T00:00:17Z

starfish/core/imagestack/imagestack.py

@@ -299,7 +300,7 @@ def from_numpy(
            cls,
            array: np.ndarray,
            index_labels: Optional[Mapping[Axes, Sequence[int]]]=None,
-            coordinates: Optional[xr.DataArray]=None,
+            coordinates: Optional[DataArrayCoordinates]=None,


parameter docs not updated below.

starfish/core/imagestack/imagestack.py

ttung · 2019-05-14T00:01:42Z

starfish/core/imagestack/parser/numpy/__init__.py

+                    float(self.coordinates[coordinate_type][0]),
+                    float(self.coordinates[coordinate_type][-1]))
+            # For the z coordinate, take the average.
+            average_z_coord = np.average(self.coordinates[Coordinates.Z])


If you pass this a full 5D tensor (i.e., where z has many layers) into from_numpy, this will mess up your z coordinates.

ttung · 2019-05-14T17:09:20Z

starfish/core/imagestack/imagestack.py

@@ -310,7 +311,7 @@ def from_numpy(
        index_labels : Optional[Mapping[Axes, Sequence[int]]]
            Mapping from axes (r, ch, z) to their labels.  If this is not provided, then the axes
            will be labeled from 0..(n-1), where n=the size of the axes.
-        coordinates : Optional[xr.DataArray]
+        coordinates : Optional[DataArrayCoordinates]
            DataArray indexed by r, ch, z, with xmin, xmax, ymin, ymax, zmin, zmax as columns.  If


This needs to be updated as well, presumably.

contract is here. actually it's not even up to date. :/

yea this is def not true anymore. it should just be DataArrayCoordinates object

ttung · 2019-05-14T17:13:49Z

starfish/core/imagestack/imagestack.py

+        physical_coords = self.xarray.coords
+        if Axes.ZPLANE in dims:
+            # if we max proj by z, take average coord
+            physical_coords[Coordinates.Z].values[0] = \


don't calculate the average twice.

also, the physical_coords[Coordinates.Z] is the wrong size! It should be size = 2, according to the contract for from_numpy (min, max).

furthermore, the size of the coords for X/Y is also wrong. It's currently the size of the X/Y, and presumably it ought to also be 2.

average_z_coordinate = np.average((self.xarray.coords[Coordinates.Z]) physical_coords[Coordinates.Z] = xr.DataArray(...)

Furthermore, if one max projects across another dimension where we assign coordinates (like x or y), we would presumably need to do the same thing. Alternatively, we could explicitly disallow max projecting across x or y, but if we want to do that, we should clear it with Deep/Ambrose/Kevin/Brian.

There is no min/max contract in from_numpy, it takes the first and last values from the array. So they can be any size

But I can do that here if you think that's cleaner

This is based on top of #1352 and #1291. The most likely path for this to get merged is for #1352 (and its supporting PRs) to land, rebase #1291 on top of master, and merging this all in.

ttung · 2019-05-24T17:19:54Z

Going to close this as it landed with #1353

fixing coordinate issues with max_proj

e86fe66

shanaxel42 requested a review from ttung April 29, 2019 20:25

ttung requested changes Apr 29, 2019

View reviewed changes

removing dead code

b40213d

shanaxel42 requested a review from ttung April 30, 2019 17:11

ttung requested changes Apr 30, 2019

View reviewed changes

adding new test

8723a86

Shannon Axelrod added 3 commits April 30, 2019 13:08

fix import error

527eaa1

changes.

ab8bbc5

changing z coord in max proj to average

eef31b9

shanaxel42 requested a review from ttung May 7, 2019 23:51

ttung mentioned this pull request May 13, 2019

Add Filter.Reduce (general dimension reduction for ImageStack) #1342

Merged

2 tasks

fixing weird notebook diffs

b6f2ecc

ttung requested changes May 14, 2019

View reviewed changes

review fixes

6f9a167

shanaxel42 requested a review from ttung May 14, 2019 14:05

ttung requested changes May 14, 2019

View reviewed changes

get rid of repeated average

cbddb91

shanaxel42 requested a review from ttung May 14, 2019 19:10

ttung mentioned this pull request May 15, 2019

Rework physical coordinate calculation for max_proj #1353

Merged

ttung closed this May 24, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixing coordinate issues with max_proj #1291

Fixing coordinate issues with max_proj #1291

shanaxel42 commented Apr 29, 2019

ttung commented Apr 29, 2019

shanaxel42 commented Apr 29, 2019 •

edited

Loading

codecov-io commented Apr 29, 2019 •

edited

Loading

ttung commented Apr 29, 2019

ttung left a comment

ttung Apr 29, 2019

shanaxel42 Apr 29, 2019

shanaxel42 commented Apr 29, 2019

neuromusic commented Apr 30, 2019

ttung commented Apr 30, 2019

ttung left a comment

shanaxel42 commented Apr 30, 2019

ttung commented Apr 30, 2019

kevinyamauchi commented May 13, 2019

shanaxel42 commented May 13, 2019

kevinyamauchi commented May 13, 2019

ttung May 14, 2019

ttung May 14, 2019

ttung May 14, 2019

ttung May 14, 2019

shanaxel42 May 14, 2019

ttung May 14, 2019

shanaxel42 May 14, 2019

shanaxel42 May 14, 2019

ttung commented May 24, 2019

Fixing coordinate issues with max_proj #1291

Fixing coordinate issues with max_proj #1291

Conversation

shanaxel42 commented Apr 29, 2019

ttung commented Apr 29, 2019

shanaxel42 commented Apr 29, 2019 • edited Loading

codecov-io commented Apr 29, 2019 • edited Loading

Codecov Report

ttung commented Apr 29, 2019

ttung left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shanaxel42 commented Apr 29, 2019

neuromusic commented Apr 30, 2019

ttung commented Apr 30, 2019

ttung left a comment

Choose a reason for hiding this comment

shanaxel42 commented Apr 30, 2019

ttung commented Apr 30, 2019

kevinyamauchi commented May 13, 2019

shanaxel42 commented May 13, 2019

kevinyamauchi commented May 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ttung commented May 24, 2019

shanaxel42 commented Apr 29, 2019 •

edited

Loading

codecov-io commented Apr 29, 2019 •

edited

Loading