Limit max tracks via track-local queues #1447
Conversation
…almolab/sleap into shrivaths/fix-maximum-tracking
Codecov Report
@@ Coverage Diff @@
## develop #1447 +/- ##
===========================================
+ Coverage 73.04% 73.12% +0.07%
===========================================
Files 134 134
Lines 23851 23953 +102
===========================================
+ Hits 17423 17516 +93
- Misses 6428 6437 +9
... and 2 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
WalkthroughThis pull request introduces enhancements to the tracking functionality in SLEAP, including options for maximum tracking and maximum number of tracks. It also includes corresponding updates to the CLI and tests to ensure the new features work as expected. Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 4
Files selected for processing (5)
- sleap/nn/inference.py (4 hunks)
- sleap/nn/tracking.py (20 hunks)
- tests/nn/test_inference.py (4 hunks)
- tests/nn/test_tracker_components.py (2 hunks)
- tests/nn/test_tracking_integration.py (3 hunks)
Additional comments (Suppressed): 20
tests/nn/test_inference.py (3)
1343-1353: The test function
test_retrackinghas been updated to include two new tracker methods: "simplemaxtracks" and "flowmaxtracks". Ensure that the corresponding functionality for these new methods is implemented correctly.1362-1370: The command line arguments
--tracking.max_tracking 1and--tracking.max_tracks 2are added when the tracker method is either "simplemaxtracks" or "flowmaxtracks". Make sure these arguments are handled properly in the main application.1495-1549: A new test function
test_max_tracks_matching_queuehas been added to test the generation of flow max tracks instances. The test seems comprehensive and checks various aspects such as track window size, number of generated tracks, etc. This is a good addition to ensure the correct functioning of the new feature.tests/nn/test_tracker_components.py (1)
- 18-22: The new trackers "simplemaxtracks" and "flowmaxtracks" are added to the parameterized test. Ensure that these trackers are implemented correctly and their behavior is as expected.
sleap/nn/inference.py (4)
71-71: The import statement for
make_scoped_dictionaryhas been updated to include the full path. Ensure that all calls to this function throughout the codebase have been updated to match the new import.4776-4776: The description of
tracker_max_instanceshas been changed from "discard instances beyond this count when tracking" to "create at most this many tracks". This seems to indicate a change in functionality. Please verify if this is the intended behavior and if it aligns with the rest of the system's logic.4870-4877: The
Tracker.make_tracker_by_namefunction call has been updated with new argumentsmax_trackingandmax_tracks. Ensure that these changes are reflected in the function definition and that they don't break existing functionality.5344-5344: The function call
sleap.util.make_scoped_dictionary(vars(args), exclude_nones=True)has been replaced withmake_scoped_dictionary(vars(args), exclude_nones=True). Ensure that all calls to this function throughout the codebase have been updated to match the new import.tests/nn/test_tracking_integration.py (2)
95-100: The new trackers
SimpleMaxTrackerandFlowMaxTrackerhave been added to the dictionary of available trackers. This change is in line with the PR's goal of introducing a feature to limit the number of tracks generated during inference-based tracking.157-184: The main function has been updated to handle the new
SimpleMaxTrackerandFlowMaxTrackerclasses. For these new trackers, the function creates a tracker instance withmax_tracks=2andmax_tracking=True. ForFlowMaxTracker, it also tries multiple scales. This change aligns with the PR's goal of enhancing tracking functionality and flexibility.sleap/nn/tracking.py (10)
92-100: The new class
MatchedFrameInstanceis introduced here. Ensure that all references to this class throughout the codebase have been updated accordingly.142-200: This hunk introduces two new methods
get_shifted_instances_from_earlier_timeandget_shifted_instances. These methods are used for generating shifted instances from earlier time and returning a list of shifted instances respectively. Make sure these methods are correctly implemented and called in the rest of the codebase.358-390: The new class
FlowMaxTracksCandidateMakeris introduced here. This class is responsible for producing optical flow shift matching candidates with maximum tracks. Ensure that all references to this class throughout the codebase have been updated accordingly.461-481: The new class
SimpleMaxTracksCandidateMakeris introduced here. This class is responsible for generating instances with maximum number of tracks from prior frames. Ensure that all references to this class throughout the codebase have been updated accordingly.562-568: New attributes
max_tracksandmax_trackingare added to theTrackerclass. Ensure that these attributes are properly initialized and used in the rest of the codebase.724-736: This hunk adds tracked instances to the dictionary of matched instances if
max_trackingis enabled. The logic seems correct, but ensure that the dictionarytrack_matching_queue_dictis properly managed in the rest of the codebase.835-843: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [810-903]
In the
make_tracker_by_namemethod, the parametersmax_tracksandmax_trackingare now passed to theTrackerconstructor. Ensure that these parameters are correctly handled in theTrackerclass.
932-940: New options
max_trackingandmax_tracksare added to theget_optionsmethod. Ensure that these options are correctly handled when they are used.1089-1098: The new class
FlowMaxTrackeris introduced here. This class is a pre-configured tracker to use optical flow shifted candidates with max tracks. Ensure that all references to this class throughout the codebase have been updated accordingly.1110-1118: The new class
SimpleMaxTrackeris introduced here. This class is a pre-configured tracker to use simple, non-image-based candidates with max tracks. Ensure that all references to this class throughout the codebase have been updated accordingly.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Files selected for processing (3)
- sleap/nn/inference.py (4 hunks)
- tests/nn/test_tracker_components.py (2 hunks)
- tests/nn/test_tracking_integration.py (3 hunks)
Additional comments (Suppressed): 11
sleap/nn/inference.py (4)
71-71: The import of
make_scoped_dictionaryhas been moved fromsleap.utilto the local module. Ensure that this function is correctly implemented in the local module and all its dependencies are properly handled.4776-4776: The description for
tracker_max_instanceshas been changed from "discard instances beyond this count when tracking" to "create at most this many tracks". This seems to indicate a change in functionality. Please verify if this change aligns with the intended behavior of limiting the number of tracks generated during inference-based tracking.4865-4877: The logic for appending "maxtracks" to the tracker name and creating the tracker has been updated. The new code checks if
tracker_max_instancesis notNoneand appends "maxtracks" to the tracker name if it isn't already there. Then, it creates the tracker usingTracker.make_tracker_by_name, passing in the updated tracker name and the new parametersmax_trackingandmax_tracks. This looks like a good implementation of the feature to limit the number of tracks generated.5344-5344: The call to
make_scoped_dictionarynow uses the local module instead ofsleap.util. As mentioned earlier, ensure that this function is correctly implemented in the local module and all its dependencies are properly handled.tests/nn/test_tracking_integration.py (2)
95-102: The addition of
SimpleMaxTrackerandFlowMaxTrackerto the dictionary of trackers is a good way to extend the functionality of the tracking system. This allows for limiting the number of tracks generated during inference-based tracking.157-184: The main function has been updated to handle the new tracker classes
SimpleMaxTrackerandFlowMaxTracker. For these trackers, the function creates a tracker with a maximum of 2 tracks and enables maximum tracking. For theFlowMaxTracker, it also tries multiple scales. This is a good way to test the new functionality introduced by the PR.tests/nn/test_tracker_components.py (5)
18-22: The new trackers "simplemaxtracks" and "flowmaxtracks" are added to the parameterized test. Ensure that these trackers are implemented correctly and their behavior is as expected.
173-188: This function
make_instsgenerates instances for testing. It seems to be well-implemented, but ensure that it's used correctly in the tests below.191-256: These lines introduce a new test
test_max_tracking_large_gap_single_track. The test first checks the tracking with a simple tracker and then with a simple max tracks tracker. The assertions check if the number of tracks matches the expected values. This test seems to be well-implemented, but ensure that theTracker.make_tracker_by_namemethod handles the "simplemaxtracks" tracker correctly.259-320: These lines introduce another new test
test_max_tracking_small_gap_on_both_tracks. Similar to the previous test, it first checks the tracking with a simple tracker and then with a simple max tracks tracker. The assertions check if the number of tracks matches the expected values. This test seems to be well-implemented, but ensure that theTracker.make_tracker_by_namemethod handles the "simplemaxtracks" tracker correctly.323-389: These lines introduce yet another new test
test_max_tracking_extra_detections. Similar to the previous tests, it first checks the tracking with a simple tracker and then with a simple max tracks tracker. The assertions check if the number of tracks matches the expected values. This test seems to be well-implemented, but ensure that theTracker.make_tracker_by_namemethod handles the "simplemaxtracks" tracker correctly.
| tracker_name, matcher_name, sim_name, max_tracks, max_tracking=False, scale=0 | ||
| ): | ||
| if tracker_name == "simplemaxtracks" or tracker_name == "flowmaxtracks": | ||
| tracker = tracker[tracker_name]( | ||
| matching_function=matchers[matcher_name], | ||
| similarity_function=similarities[sim_name], | ||
| max_tracks=max_tracks, | ||
| max_tracking=max_tracking, | ||
| ) | ||
| else: | ||
| tracker = trackers[tracker_name]( | ||
| matching_function=matchers[matcher_name], | ||
| similarity_function=similarities[sim_name], | ||
| ) | ||
| if scale: | ||
| tracker.candidate_maker.img_scale = scale | ||
| return tracker |
There was a problem hiding this comment.
The make_tracker function has been updated to handle the new tracker classes SimpleMaxTracker and FlowMaxTracker. The function now takes two additional parameters: max_tracks and max_tracking. These parameters are used when creating instances of the new tracker classes. However, there's a typo in line 119 where you're trying to access tracker instead of trackers.
- tracker = tracker[tracker_name](
+ tracker = trackers[tracker_name](
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Files selected for processing (1)
- tests/nn/test_tracking_integration.py (4 hunks)
Additional comments (Suppressed): 5
tests/nn/test_tracking_integration.py (5)
12-23: The
test_simple_trackerfunction is well implemented and tests the functionality of the simple tracker. It checks if the number of tracks generated by the tracker matches the expected number.25-36: The
test_simplemax_trackerfunction tests the functionality of the newSimpleMaxTracker. It verifies that the maximum number of tracks does not exceed the specified limit. This is a good test to ensure that the new feature works as expected.130-131: The addition of
SimpleMaxTrackerandFlowMaxTrackerto the dictionary of trackers allows these new trackers to be used in the testing framework. This is a necessary change for testing the new features.148-164: The
make_trackerfunction has been updated to handle the new trackers. The function now takes two additional parameters,max_tracksandmax_tracking, which are used when creating instances ofSimpleMaxTrackerandFlowMaxTracker. This is a good update to accommodate the new features.189-216: This section of code has been updated to handle the new trackers when running the tests. For each combination of tracker, matcher, similarity, and scale, it creates an instance of the tracker and runs the test function on it. The changes are well implemented and necessary for testing the new features.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Files ignored due to filter (1)
- sleap/config/pipeline_form.yaml
Files selected for processing (1)
- sleap/gui/learning/runners.py (3 hunks)
Additional comments (Suppressed): 3
sleap/gui/learning/runners.py (3)
224-230: The new parameter
tracking.max_trackshas been added to the list of optional items. Ensure that this parameter is handled correctly in all places whereoptional_items_as_nonesis used.238-245: This block of code introduces a new tracking mode,
max_tracking, which is activated when thetracking.max_tracksparameter is present. It also modifies the tracker name by appending "maxtracks" if the current tracker is either "simple" or "flow". This could potentially cause confusion or errors if other parts of the code are not expecting these modified tracker names. Verify that this change is compatible with the rest of the codebase.253-258: The new parameter
tracking.max_trackinghas been added to the list of boolean items. Ensure that this parameter is handled correctly in all places wherebool_items_as_intsis used.
talmo
changed the title
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Files selected for processing (1)
- docs/guides/cli.md (1 hunks)
Additional comments (Suppressed): 4
docs/guides/cli.md (4)
118-129: The new tracking options
--tracking.max_trackingand--tracking.max_trackshave been added to the CLI documentation. Please ensure that these options are correctly implemented in the codebase and function as expected.185-188: ```diff
- --tracking.tracker TRACKING.TRACKER
Options: simple, flow, None (default: None)
- --tracking.tracker TRACKING.TRACKER
Options: simple, flow, simplemaxtracks, flowmaxtracks, None (default: None)The tracker options have been updated to include `simplemaxtracks` and `flowmaxtracks`. This change should be reflected in all parts of the code where the tracker option is used. * 259-263: This example demonstrates how to use the new max tracks limit feature. It's a good addition to the documentation as it helps users understand how to use the new feature. * 267-269: This example shows how to re-track without pose inference using the new max tracks limit feature. It's a useful addition for users who want to apply the new feature to existing predictions. </blockquote></details></blockquote></details> </details>
commit 734283a Author: Liezl Maree <38435167+roomrys@users.noreply.github.com> Date: Sun Sep 10 08:19:36 2023 -0700 Bump to 1.3.2 (#1482) * Bump to 1.3.2 * Run manual build w/o pip upload * Use `importlib.resources` if available * Rebuild mac conda package * Lint * Build/upload pip wheel * Reset build_manual and up build number commit e424501 Author: Liezl Maree <38435167+roomrys@users.noreply.github.com> Date: Sat Sep 9 10:32:29 2023 -0700 Add pip extras (#1481) * Rename pip to pypi, add tensorflow * Move out jupyter requirements from dev * Add extras for conda/pip jupyter and dev * Rename `pip` extra to `pypi` * Add build_ci workflow * Install pip package using extras * Rerun notebooks with new pip wheel * Internal import after adding relative path to sys * Build develop docs on this branch * Add comments to setup.py extras * Update installation docs * Add wget bypass for apple silicon mambaforge * Create func to combine req * Italicize jupyter instead of bold --------- Co-authored-by: modularizer <modularizer@gmail.com> Co-authored-by: roomrys <> commit 93ef288 Author: Shrivaths Shyam <52810689+shrivaths16@users.noreply.github.com> Date: Sat Sep 9 03:12:58 2023 +0530 Limit max tracks via track-local queues (#1447) * Initial commit * format files * [wip] adding local deque for tracks * format files * [wip] adding local deque for tracks * [wip] Add max tracking for simpletracker * [wip] Add max tracking for simple tracker * [wip] add missing argument * [wip] Add and modify test functions * [wip] Add and modify test functions * Bug fix and refactoring code * [wip] Add max tracking for flow tracker. * [wip] Including suggested changes * [wip] refactor code * Add test function to check max tracks * Added suggestions and feedback * Prevent the creation of more than max tracks when we have unmatched detections * Add tests * Use maximum tracking by default when loading model via high level API * Lint * Fix integration test * Refactor max tracker tests * Add integration test for CLI * typo * Add max tracks to the tracking GUI * Update CLI docs and add examples --------- Co-authored-by: Talmo Pereira <talmo@princeton.edu> Co-authored-by: Talmo Pereira <talmo@salk.edu> commit 64655d6 Author: DivyaSesh <64513125+gitttt-1234@users.noreply.github.com> Date: Wed Sep 6 09:50:35 2023 -0700 Fix Auto-select GPU (#1474) * Fix Auto-select GPU * Format file * Add variable in init * Format files * Add small test to ensure environment variable is set * Make linter happy --------- Co-authored-by: roomrys <lmaree@ucsd.edu> commit 0ef52cd Author: Liezl Maree <38435167+roomrys@users.noreply.github.com> Date: Thu Aug 17 11:03:51 2023 -0700 Migrate to `importlib_resources` backport (#1458) * Switch to backport * Remove `pkg_resources` * Clean-up function (non-logical) * Make linter happy * Fix-up path for last few stragglers * Use `Path.as_posix` method instead of `str` commit e0eebb2 Author: Liezl Maree <38435167+roomrys@users.noreply.github.com> Date: Tue Aug 15 13:10:06 2023 -0700 Handle error message edge case when finding yaml paths (#1456) commit 6858563 Author: Liezl Maree <38435167+roomrys@users.noreply.github.com> Date: Fri Aug 11 11:33:14 2023 -0700 Add message if drag drop fails (#1451) * Fix drag and drop * Feedback when user drops invalid file type * Change wording on message commit 88fdb68 Author: Liezl Maree <38435167+roomrys@users.noreply.github.com> Date: Fri Aug 11 11:27:57 2023 -0700 Pin `tensorflow-hub<0.14.0` (#1446) * Pin pynwb 2.3.3 * Remove pynwb pin, add comments commit 4730788 Author: Talmo Pereira <talmo@salk.edu> Date: Fri Aug 11 10:52:09 2023 -0700 Fix drag and drop (#1449) commit 47f8096 Author: DivyaSesh <64513125+gitttt-1234@users.noreply.github.com> Date: Thu Aug 10 16:29:39 2023 -0700 Add Option to Export CSV (#1438) * Add Option to Export CSV * Add Test Functions * Fomat Files * Change FormatID commit 5ba6bc1 Author: Liezl Maree <38435167+roomrys@users.noreply.github.com> Date: Thu Aug 10 10:17:45 2023 -0700 Add model folder to the unzip path (#1445) * Add model folder to the unzip path * Handle cases where zipped model either has no extra directory * Add test * Fix-up test and implementation * Manually lint commit d61a184 Author: KevinZ0217 <96039456+KevinZ0217@users.noreply.github.com> Date: Wed Aug 9 14:47:21 2023 -0700 Change the hotkey for exporting h5 analysis (#1444) * Change the hotkey for export h5 files to Ctrl+Alt+E commit ad7529e Author: KevinZ0217 <96039456+KevinZ0217@users.noreply.github.com> Date: Wed Aug 9 14:46:09 2023 -0700 Add a button for copying model config to clipboard (#1433) * Add shortcur for export_analysis_current * Fix the linting issue * Add the button without copy method' * Add button for copying model config to clipboard * Fix linting by reformatting * Use Qtpy for clipboard rather than pyperclip * Pretty print model config json to clipboard and fix missing command * Fix the overwriting problem for dict object * Delete unnecessary print statement * Add a few comments & Remove unnecessary variables & remove unused function commit 2611e7d Author: Liezl Maree <38435167+roomrys@users.noreply.github.com> Date: Wed Aug 9 11:52:57 2023 -0700 Improve error message for detecting video backend (#1441) * Improve error message for detecting video backend * Lint * Add small test commit 1151a95 Author: Shrivaths Shyam <52810689+shrivaths16@users.noreply.github.com> Date: Wed Aug 2 17:08:38 2023 -0700 Fix error thrown when last video is deleted (#1421) * Handle None Video case during callbacks * format files * remove unused comments * Disable remove video button when there are no videos * Display default background when all videos are removed * Format files * Remove overlay error after removing last video * Redraw overlays on plot change (#1435) * Redraw overlays after changedPlot, changedPlot on reset * Update instance state on player reset --------- Co-authored-by: Liezl Maree <38435167+roomrys@users.noreply.github.com> commit 6002332 Author: Shrivaths Shyam <52810689+shrivaths16@users.noreply.github.com> Date: Mon Jul 31 13:28:53 2023 -0700 Update status message on status bar (#1411) * Update status message on status bar * Update statusbar to show correct video count * remove additional conditional check commit 3a01ef3 Author: Liezl Maree <38435167+roomrys@users.noreply.github.com> Date: Mon Jul 31 12:09:50 2023 -0700 Correct GUI state emulation (#1422) commit e94b516 Author: Liezl Maree <38435167+roomrys@users.noreply.github.com> Date: Thu Jul 27 12:11:41 2023 -0700 Add video path and frame indices to metrics (#1396) * Add `Instance`s and `PredictedInstance`s to metrics * Add tests * Add frame/video info to metrics, wip: test writing * Fix metrics save test commit b2ad203 Author: Liezl Maree <38435167+roomrys@users.noreply.github.com> Date: Thu Jul 27 12:08:19 2023 -0700 Fix labels export for json (#1410) * wip: fix labels export for json * Add test for json.zip labels pkg * Add test for .slp labels pkg * Make linter happy commit 90c012d Author: KevinZ0217 <96039456+KevinZ0217@users.noreply.github.com> Date: Wed Jul 26 13:30:22 2023 -0700 Add shortcut to export analysis for current video (#1414) * Add shortcur for export_analysis_current * Fix the linting issue commit f9d0a22 Author: Shrivaths Shyam <52810689+shrivaths16@users.noreply.github.com> Date: Tue Jul 25 09:37:52 2023 -0700 Modify compute OKS function (#1399) * Update compute OKS function * Update compute OKS function * Modify compute OKS function * Added suggested changes * Added further suggestions and comments * Added the permalink to the cocoeval function * Added permalink to cocoeval function --------- Co-authored-by: Liezl Maree <38435167+roomrys@users.noreply.github.com> Co-authored-by: Talmo Pereira <talmo@salk.edu> commit 904338c Author: Liezl Maree <38435167+roomrys@users.noreply.github.com> Date: Tue Jul 25 06:34:25 2023 -0700 Add `Video` to cache when adding `Track` (#1407) * Add `Video` to cache when adding `Track` * Use methods instead of rewriting code * Simplify code commit 845214c Author: DivyaSesh <64513125+gitttt-1234@users.noreply.github.com> Date: Mon Jul 24 21:20:02 2023 -0700 Fix Remove Videos in Batch (#1406) * Fix Remove Videos in Batch * Remove Unused Testing Code commit 0afbb9b Author: Liezl Maree <38435167+roomrys@users.noreply.github.com> Date: Mon Jul 24 17:37:42 2023 -0700 Add `Track` when add `Instance` (#1408) commit d173303 Author: Liezl Maree <38435167+roomrys@users.noreply.github.com> Date: Mon Jul 24 08:25:09 2023 -0700 Fix skeleton templates (#1404) commit 0e7a372 Author: DivyaSesh <64513125+gitttt-1234@users.noreply.github.com> Date: Fri Jul 21 09:49:51 2023 -0700 Fix panning bounding box (#1398) commit fb61b6c Author: Liezl Maree <38435167+roomrys@users.noreply.github.com> Date: Wed Jul 19 15:30:43 2023 -0700 Fix `Filedialog` to work across (mac)OS (#1393) * Always use dir instead of directory * Wrap `FileDialog` methods for OS-specific calls * Clean-up os-specific wrapper to check for linux only * Lint * Fix test for non native `FileDialog` commit 19cd2b5 Author: DivyaSesh <64513125+gitttt-1234@users.noreply.github.com> Date: Mon Jul 17 17:55:29 2023 -0700 Add option to remove videos in batch (#1382) * add option to remove videos in batch * Add option to remove videos in batch * Add option to remove videos in batches * Modify Lint format * Add Test cases * Modify Test Cases for Removing Videos in Batch * Add Comment to test_docks * Remove commented line * Format files commit 3b5c4ff Author: Shrivaths Shyam <52810689+shrivaths16@users.noreply.github.com> Date: Fri Jul 14 17:06:58 2023 -0700 Minor fix in computation of OKS (#1383) * fix compute oks * Update the oks fix commit 1c2be11 Author: Liezl Maree <38435167+roomrys@users.noreply.github.com> Date: Thu Jul 6 14:44:01 2023 -0700 Pin micromamba version (#1376) * Pin micromamba version * Add build number to pin
Description
While performing inference based tracking, the number of tracks that are generated needs to be capped in order to avoid creating multiple irrelevant/redundant tracks.
We implement a new mode of tracking here to address this by enabling track-local queues to store candidates for matching. This allows for setting the global maximum number of tracks, irrespective of how large the gaps between detections of a track are.
This also exposes that functionality to the
load_modelAPI, the CLI, and the GUI, where now superseded options such as pre-tracking culling have been removed.Types of changes
Does this address any currently open issues?
Outside contributors checklist
Thank you for contributing to SLEAP!
❤️
Summary by CodeRabbit
Trackerclass, enhancing its flexibility and functionality.load_modeland_make_tracker_from_clifunctions.--tracking.max_tracking,--tracking.max_tracks, and--tracking.robust.max_trackingandmax_tracksparameters.