Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: Prediction fails on empty frames #367

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nkeim
Copy link
Contributor

@nkeim nkeim commented May 14, 2016

This adds a test that points out an unwanted behavior: prediction fails on empty frames. It should at least print an informative error message; instead we just get an IndexError when link() tries to read the frame number by inspecting the first particle.

I am leaving this open without a fix for now, because I'd like to revisit it when I/we finish #333.

@pfigliozzi
Copy link
Contributor

I ran into this issue when trying to link some particles into trajectories. I have stretches where the particles disappear from the frame for 10 frames or more which is much greater than my memory setting (memory=3). I fixed it by changing linking.py. From the current master:

if self.predictor is not None:
    # This only works for KDTree right now, because KDTree can store particle
    # positions in a separate data structure from the PointND instances.
    if not isinstance(prev_hash, TreeFinder):
         raise NotImplementedError(
                        'Prediction works with the "KDTree" neighbor_strategy only.')
    # Get the time of cur_level from its first particle
    t_next = list(itertools.islice(cur_level, 0, 1))[0].t
    targeted_predictor = functools.partial(self.predictor, t_next)
    prev_hash.rebuild(coord_map=targeted_predictor) # Rewrite positions

In empty frames cur_level is an empty list. So instead I use the prev_level and add one to it. It seems to work for me. As a band aid I changed it with a try: except: in order to keep the original code intact as possible. Here is what it looks like in my local version:

            if self.predictor is not None:
                # This only works for KDTree right now, because KDTree can store particle
                # positions in a separate data structure from the PointND instances.
                if not isinstance(prev_hash, TreeFinder):
                    raise NotImplementedError(
                        'Prediction works with the "KDTree" neighbor_strategy only.')
                # Get the time of cur_level from its first particle
                try:
                    t_next = list(itertools.islice(cur_level, 0, 1))[0].t
                except IndexError:
                    t_next = list(itertools.islice(prev_level, 0, 1))[0].t + 1
                targeted_predictor = functools.partial(self.predictor, t_next)
                prev_hash.rebuild(coord_map=targeted_predictor) # Rewrite positions

It seems to work fine for my purposes perhaps because the length of time that the particles spend out of frame is longer than my current memory setting (which is fine with me).

Also this might be a naive question but I don't understand the point of using list(itertools.islice(cur_level, 0, 1))[0].t anyway. If cur_level is just a list of Point objects and you want the first one why not just use cur_level[0].t?

@nkeim
Copy link
Contributor Author

nkeim commented Feb 4, 2017

Thanks, @pfigliozzi ! I think that you want to use prev_set since that is what actually gets updated. As far as I can tell (and this is embarassing), prev_level will always contain the features from the first frame of the movie.

But, taking a step back, I'm not sure that the value of t_next matters at all here. Since no links can be made to the empty level, the coordinates of the particles are irrelevant. Could we instead just skip the entire prev_hash.rebuild sequence if there's an empty frame? The predictor will know what to do when particles appear again.

The levels are usually lists, but for purposes of link() they only need to be iterables. So I suppose we were trying to honor that (I can't quite recall). The particular islice construction is due to @caspervdw.

@caspervdw
Copy link
Member

I really don't remember coming up with this islice 😕, but I agree that it is because prev_level is allowed to be non-indexable.

Anyway, thanks for this suggestion @pfigliozzi. I think that this issue will be solved in the new linking implementation (over here https://github.com/soft-matter/trackpy/pull/414/files#diff-ef01253517b496f9c55faa898b68174bR693) as the linker is 'frame-number aware' anyway.

We do have to finish the new linking implementation (currently stuck at PR #414).
@nkeim, maybe we could have a quick skype session sometime to get that going?

@nkeim
Copy link
Contributor Author

nkeim commented Feb 5, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants