kymotrack: add `sample_from_channel` #685

JoepVanlier · 2024-08-02T16:16:15Z

Why this PR?
It can be pretty error prone for users to do this themselves. To do this properly, they have to index line_timestamp_ranges() themselves and have to make sure the kymograph they apply that on is the one they have tracks of (and that it is cropped in the same way).

Docs build here and here.

mikhas · 2024-08-05T10:08:09Z

docs/tutorial/kymotracking.rst

+    track_force = longest_track.sample_from_channel(force_slice, include_dead_time=True)
+
+When you call this function with a :class:`~lumicks.pylake.Slice`, it returns another :class:`~lumicks.pylake.Slice` with the downsampled channel data.
+What happens is that for every point on the track, the correct kymograph scan line is looked up and the channel data is averaged over the entire duration of that scan line.


Would clarify. It's either the entire length of the scan line or the entire duration of the line scanning.

As in: the scan line is already an artificial construct where the time it took to record it has been factored in, so talking about duration here again is potentially confusing.

Isn't the difference already clarified by the line immediately after?

The parameter include_dead_time specifies whether the time it takes the mirror to return to its initial position after each scan line should be included in this average.

What do you think it should read?

mikhas · 2024-08-05T10:15:38Z

lumicks/pylake/kymotracker/tests/test_image_sampling.py

+    )
+
+    data = Slice(Continuous(np.arange(100), start=_FIRST_TIMESTAMP, dt=int(1e8)))  # 10 Hz
+    kymotrack = KymoTrack(time_idx, [0, 1, 2], kymo, "red", 0)


Lacks the case where the queried time indices are larger than the given track points, e.g., time_idx = [0, 1, 2, 3] (or even [0, 3], [2, 3]). I am guessing that sample_from_channel would fail and return an empty slice but yield no warning to the user, correct?

Or is that the scenario you test for in the test_sample_from_channel_no_overlap test? Note: It's not clear to me what no_overlap refers to in that test.

If a time index outside the Kymo is required, it will raise. I don't really see how this would be possible in a real scenario though, since the KymoTrack is derived from the Kymo being sampled and the Kymo is private and cannot be modified.

The no overlap case is the case where the channel data has no overlap with the kymotrack (which would happen if you happen to evaluate it for the wrong slice for example). It is important to handle that case, since it is not unthinkable that a user might try to match channels from the wrong file with another kymo.

I can add a new test for it, but I am not sure whether we need dedicated error handling for this because in this case I would like to hear about it and the stack trace would probably be more informative than our handling.

mikhas · 2024-08-05T10:25:33Z

lumicks/pylake/kymotracker/kymotrack.py

+        ts_ranges = self._kymo.line_timestamp_ranges(include_dead_time=include_dead_time)
+        try:
+            return channel_slice.downsampled_over(
+                [ts_ranges[time_idx] for time_idx in self.time_idx]


How is it guaranteed that each available time index also exists in ts_ranges, especially if include_dead_time is False? Wouldn't ts_ranges then miss a couple of indices that exist in self.time_idx?

Why would include_dead_time matter? Include dead time impacts the duration of each timestamp range interval, not the number of ranges that are there.

~~Don't you get a different set of indices, depending on that flag?~~ Nope you don't, I misunderstood.

rpauszek

This is a really nice convenience feature. I left some suggestions for wording and I think it's a good idea to include an optional reduce argument like we do for other downsampling methods

rpauszek · 2024-09-18T08:40:06Z

docs/tutorial/kymotracking.rst

+    track_force = longest_track.sample_from_channel(force_slice, include_dead_time=True)
+
+When you call this function with a :class:`~lumicks.pylake.Slice`, it returns another :class:`~lumicks.pylake.Slice` with the downsampled channel data.
+What happens is that for every point on the track, the correct kymograph scan line is looked up and the channel data is averaged over the entire duration of that scan line.


Suggested change

What happens is that for every point on the track, the correct kymograph scan line is looked up and the channel data is averaged over the entire duration of that scan line.

For every point on the track, the corresponding kymograph scan line is looked up and the channel data is averaged over the entire duration of that scan line.

rpauszek · 2024-09-18T08:41:27Z

docs/tutorial/kymotracking.rst

+
+When you call this function with a :class:`~lumicks.pylake.Slice`, it returns another :class:`~lumicks.pylake.Slice` with the downsampled channel data.
+What happens is that for every point on the track, the correct kymograph scan line is looked up and the channel data is averaged over the entire duration of that scan line.
+The parameter `include_dead_time` specifies whether the time it takes the mirror to return to its initial position after each scan line should be included in this average.


Suggested change

The parameter `include_dead_time` specifies whether the time it takes the mirror to return to its initial position after each scan line should be included in this average.

The parameter `include_dead_time` specifies whether the dead time (the time it takes the mirror to return to its initial position after each scan line) should be included in this average.

rpauszek · 2024-09-18T08:42:54Z

lumicks/pylake/kymotracker/kymotrack.py

@@ -621,6 +637,34 @@ def _split(self, node):

        return before, after

+    def sample_from_channel(self, channel_slice, include_dead_time=True) -> Slice:


since this is effectively another downsampling method, I think it would be good to include an optional reduce function with np.mean as the default like we do for Slice.downsample_over()

Yeah, I was doubting about that. I'll add it.

rpauszek · 2024-09-18T08:44:23Z

lumicks/pylake/kymotracker/kymotrack.py

+        """Sample channel data using the time points corresponding to this track
+
+        For each time point in the track, sample the channel data corresponding to that kymograph
+        scan line.


Suggested change

"""Sample channel data using the time points corresponding to this track

For each time point in the track, sample the channel data corresponding to that kymograph

scan line.

"""Downsample channel data corresponding to the time points of this track

For each time point in the track, downsample the channel data over the time limits of the corresponding kymograph scan line.

JoepVanlier force-pushed the downsampled_over_track branch 3 times, most recently from 8cbeca5 to 0091360 Compare August 5, 2024 09:09

JoepVanlier marked this pull request as ready for review August 5, 2024 09:15

JoepVanlier requested review from a team as code owners August 5, 2024 09:15

JoepVanlier requested review from alessiamarcolini, tobiasjj and rpauszek August 5, 2024 09:15

mikhas reviewed Aug 5, 2024

View reviewed changes

JoepVanlier force-pushed the downsampled_over_track branch 2 times, most recently from 4122697 to 010d514 Compare August 8, 2024 14:26

JoepVanlier force-pushed the downsampled_over_track branch 2 times, most recently from 12bbc65 to cc89d1b Compare August 21, 2024 12:38

JoepVanlier force-pushed the downsampled_over_track branch from cc89d1b to 1969b09 Compare September 5, 2024 15:19

rpauszek approved these changes Sep 18, 2024

View reviewed changes

kymotrack: add sample_from_channel

45abbb5

JoepVanlier force-pushed the downsampled_over_track branch from 1969b09 to 45abbb5 Compare September 18, 2024 15:40

JoepVanlier merged commit 7b7e957 into main Sep 18, 2024
8 checks passed

JoepVanlier deleted the downsampled_over_track branch September 18, 2024 16:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kymotrack: add `sample_from_channel` #685

kymotrack: add `sample_from_channel` #685

JoepVanlier commented Aug 2, 2024 •

edited

Loading

mikhas Aug 5, 2024

JoepVanlier Aug 5, 2024

mikhas Aug 5, 2024

JoepVanlier Aug 5, 2024 •

edited

Loading

mikhas Aug 5, 2024

JoepVanlier Aug 5, 2024

mikhas Aug 5, 2024 •

edited

Loading

rpauszek left a comment

rpauszek Sep 18, 2024

rpauszek Sep 18, 2024

rpauszek Sep 18, 2024

JoepVanlier Sep 18, 2024

rpauszek Sep 18, 2024

	What happens is that for every point on the track, the correct kymograph scan line is looked up and the channel data is averaged over the entire duration of that scan line.
	For every point on the track, the corresponding kymograph scan line is looked up and the channel data is averaged over the entire duration of that scan line.

	The parameter `include_dead_time` specifies whether the time it takes the mirror to return to its initial position after each scan line should be included in this average.
	The parameter `include_dead_time` specifies whether the dead time (the time it takes the mirror to return to its initial position after each scan line) should be included in this average.

		@@ -621,6 +637,34 @@ def _split(self, node):

		return before, after

		def sample_from_channel(self, channel_slice, include_dead_time=True) -> Slice:

kymotrack: add sample_from_channel #685

kymotrack: add sample_from_channel #685

Conversation

JoepVanlier commented Aug 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoepVanlier Aug 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikhas Aug 5, 2024 • edited Loading

Choose a reason for hiding this comment

rpauszek left a 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

kymotrack: add `sample_from_channel` #685

kymotrack: add `sample_from_channel` #685

JoepVanlier commented Aug 2, 2024 •

edited

Loading

JoepVanlier Aug 5, 2024 •

edited

Loading

mikhas Aug 5, 2024 •

edited

Loading