Optimization, documentation, 3D, PBC improvements

[galexsky]

Extensive update of tobac capabilities and procedures by myself and @freemansw1 . Many substantial changes to code, such as:

• Inclusion of the vertical dimension for 3D feature detection and tracking, now accounting for a potentially non-uniform dz
• Modification to 3D segmentation allowing "box seeding" of user-set size in addition to default "column seeding" method
• Feature detection, segmentation, and tracking now able to address datasets with periodic boundary conditions (PBCs)
• Addition of several new utility functions and movement of some such functions from other modules to utils
• Optimization, simplification, bug fixes, and other streamlining of overall code base
• Improved code documentation

[freemansw1]

A few notes:

• Given the magnitude of the changes, I've created a new branch for this, called dev-3D-PBC.
• I contributed to this substantially, so I cannot review the PR. Instead, I've requested @JuliaKukulies and @w-k-jones .
• Some documentation improvements are also contained in here. I genuinely forgot that those were in here as well, so we are happy to remove them.
• #124 hasn't been merged yet, but note that coverage (of feature detection, tracking, and segmentation) has increased substantially. Once we get #124 merged, hopefully the codecov bot will run here. It still needs improvement, especially on the feature detection (and analysis) side, but this is overall better.
• There are still some TODO items, including running 3D feature detection on a subset of the points. However, @galexsky and I decided that it was better to go ahead and get this in as a PR so that we can work on these new features collaboratively.

I think I speak for both @galexsky and myself when we say that we're happy for any comments on this. This project has been a long time coming, so there are likely some bugs scattered about. Thanks also to @grleung and @smsaleeb for beta testing this.

[JuliaKukulies]

All right, thank you for your great efforts @freemansw1 and @galexsky! In my opinion, we can keep the documentation bit in this PR, as this is the part that does not take a lot of effort to review and then we have those improvements at one sweep!

As you can see, I started the review, which looks a bit unsystematic at the moment, but I thought it would be good to directly comment on minor comments and questions as well, while I am going through the code step-by-step to understand and review your additions. It will take some time to go through all changes, as I am also testing your additions with some real data. So far these extra tests look good, no problems with the automatic tests in my local clone and documentation rendering looks also fine!

[freemansw1]

I'll get to addressing your comments on this ~next week. I know Alex is quite busy finishing up for his defense at this point.

[freemansw1]

As I'm beginning to work on this, I'm realizing more and more that we should really split out the documentation stuff. 3D and PBC is a lot harder to split out, but the docs stuff should be fairly easy. If @galexsky doesn't object, I'll create a new PR for just documentation stuff and see if I can get documentation stuff out of this PR.

[JuliaKukulies]

OK, that would be great if it is not too much work you @freemansw1 @galexsky !

[codecov[bot]]

Codecov Report

Base: 41.01% // Head: 51.99% // Increases project coverage by +10.97% :tada:

Coverage data is based on head (23ceda3) compared to base (aded93d). Patch coverage: 81.09% of modified lines in pull request are covered.

Additional details and impacted files

@@              Coverage Diff               @@
##           RC_v1.5.0     #127       +/-   ##
==============================================
+ Coverage      41.01%   51.99%   +10.97%     
==============================================
  Files             14       15        +1     
  Lines           2321     3156      +835     
==============================================
+ Hits             952     1641      +689     
- Misses          1369     1515      +146     

Flag	Coverage Δ
unittests	51.99% <81.09%> (+10.97%)	:arrow_up:

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

Impacted Files	Coverage Δ
tobac/utils/__init__.py	100.00% <ø> (ø)
tobac/feature_detection.py	60.84% <45.70%> (-19.93%)	:arrow_down:
tobac/tracking.py	71.61% <83.33%> (+6.47%)	:arrow_up:
tobac/utils/general.py	71.62% <86.95%> (+7.24%)	:arrow_up:
tobac/segmentation.py	89.64% <92.18%> (+14.39%)	:arrow_up:
tobac/utils/internal.py	96.80% <94.73%> (-0.96%)	:arrow_down:
tobac/testing.py	96.29% <96.52%> (+1.73%)	:arrow_up:
tobac/utils/periodic_boundaries.py	98.78% <98.78%> (ø)
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[freemansw1]

Okay, I've done the following:

1. the branch we are merging into, dev-3d-pbc, has been updated to the latest dev.
2. As discussed, documentation changes have largely been pulled out to #150. There are some updated docstrings in here, but I will wait until #138 is merged before trying to get those all cleaned up and merged.
3. I've addressed several of the changes raised by @JuliaKukulies . I'm going to work on separating out the PBC functions into a separate utility class, but I want to do a PR addressing #122 et al. first.

[freemansw1]

I'm back to working on this now that we're very close to v1.4.0. I've got the latest changes from RC_v1.5.0 merged in, although there will be some more work to do once #190 is merged in.

I'm not ready for a re-review as there's a lot more to do, but I will ping when I am.

[JuliaKukulies]

Great @freemansw1 ! Thanks for the notification and for all your hard work on this one. I will have a look at #190 later this week.

[freemansw1]

Okay, I've updated this code for RC_V1.5.0. One thing that I'm struggling with: I cannot get our workaround for predictive tracking working with 3D data. For now, I've switched tests to use random, but that is clearly not ideal. @snilsn any ideas? I know that this will be fixed once https://github.com/soft-matter/trackpy/pull/710 is merged, but without an ETA there, it would be much better to have a workaround.

@snilsn if you're able/willing/interested in helping, I've put the "base case" in a gist here: https://gist.github.com/freemansw1/a917e0d823524c6bb44eee458e218a02 .

[snilsn]

I could get df_link_iter to produce an output by setting pos_columns to ['z', 'y', 'x'] manually, but then the prediction doesn't work. This could mean that guessing the position columns isn't working correctly for 3d inputs within df_link_iter, which would make our workaround useless. trackpy.utils.guess_pos_columns() is used for this, but seems to work fine

One idea I had was specifyng an 3d initial_guess_vels in NearestVelocityPredict, but that isn't helping.

Did you find any examples for trackpy that include tracking 3d features using predictors @freemansw1?

[snilsn]

I think there is a solution (at least theoretically). It's possible to set the values for the interpolator manually:

from scipy.interpolate import NearestNDInterpolator
import numpy as np
import pandas as pd
import trackpy

test_df = pd.DataFrame({'z': [0,1,2,3], 'y':[0,1,2,3], 
'x': [0,1,2,3], 'frame': [0,1,2,3]})

features_list = [frame for i, frame in test_df.groupby("frame", sort=True)]

initial_guess_pos = np.asarray([[0, 0, 0]])
initial_guess_vel = np.asarray([[1, 1, 1]])

pred = trackpy.predict.NearestVelocityPredict(span=1)
pred.interpolator = NearestNDInterpolator(initial_guess_pos, initial_guess_vel)

traj_iter = trackpy.link_df_iter(features_list, 
                                 search_range=0.1, 
                                 t_column='frame', 
                                 predictor=pred.predict)
track = pd.concat(traj_iter)

This code links all features into one particle despite the small search range, i. e. the prediction is happening.

The problem is, that the value for the velocity isn't adjusted automatically. This would mean that we would have to loop through all the frames in pairs, interpolate the velocity from the last frame and set a new pred.interpolator at each step, do the linking for the two frames and combine everything into one dataframe at the end.

It's probably possible, but a little ridiculous. Maybe we can iterate to something feasible from here.

[freemansw1]

I've now merged in the latest v1.5.0 changes, most importantly the utils breakout (#191). I'm in the process of thinking how best to do this. I'm becoming inclined to break out the 3D from the PBC changes, despite the pain that that will likely cause for me. As of now, this PR is extremely difficult to review due to its size and scope, with around 5K LOC changed.

I'm going to hold off asking for re-reviews until I resolve this.

[freemansw1]

It appears that, because #209 was merged and contains this commit history, GitHub has tagged this as merged as well. I'll work on a new PR for the PBC changes.

[w-k-jones]

That's a slightly annoying feature on Github! Let us know when the new PR is ready and I'll review it asap