Conversation
…wrappers Identical Cython kernels were copy-pasted across track_kernels_* modules. Consolidate them in the two modules that already have .pxd files: - pixel: _multimed_r_nlay_1layer, _point_to_pixel_out, _pixel_to_metric_out, _dist_to_flat_out, _candsearch_in_pix_rest_nogil, _sorted_candidates_fast_out_nogil, candsearch_in_pix_fast_nogil - position: _ray_tracing_out, _angle_acc_out, _point_position_out, assess_new_position_fast_nogil Other modules cimport them (pure-Python fallback imports unchanged). Also: - delete the track_kernels_tracking.py re-export shim; repoint setup.py, the cibuildwheel smoke test and the coverage tests - delete 16 kernel wrappers with no non-test callers, with their tests Status: hot-path tests passed after the dedup + shim removal. The wrapper deletions have NOT been rebuilt or tested yet, and the full suite and the pure-Python fallback check are still outstanding. See docs/plans/2026-09-20-tracking-kernels-dedup.md for next steps. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Rebuilt clean; hot-path 49 passed, full suite 2017 passed, interpreted kernel coverage tests 266 passed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Occlusion lesson: during an occlusion tracks may hold the same point, but each track's speed comes only from its own points. - two_phase_tracking.py: allow_shared/max_shared/share_tol/max_group_size (config + YAML wiring), double-claim in contested groups, velocity never updated from shared points, tid-annotated chains, giant-component Hungarian cap. - track_kernels_track3d.py (Level 1 only): contested + mutually-well-predicted edges recorded, not stolen; caller-allocated shared output arrays; cold levels never share. - tracking_shared.py (new): linkage-level mark_shared_observations + assemble_with_shared for the trackcorr path (kernel untouched). - scripts/proto_shared_validate.py, scripts/proto_long_validate.py: synthetic + 50-frame real-data harness. - docs/trackers.md (all eight engines, caveats, MyPTV/proPTV credits), docs/two-phase-tracking.md (usage + parameters), index links. All prototype flags default off. Related unit tests (60, incl. gap-relink) pass.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved compatibility and shared-observation correctness issues remain.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (7)
What changed in this PR
This PR prototypes shared-observation tracking across multiple trackers and refactors tracking kernels.
Changes:
- Adds sharing support to Two-Phase, Fast3D, and trackcorr.
- Deduplicates Cython kernels and updates tests/build configuration.
- Adds validation scripts and tracker documentation.
| File | Summary |
|---|---|
tests/unit/test_track_kernels_transform_coverage.py |
Updates transform-kernel coverage |
tests/unit/test_track_kernels_tracking_coverage.py |
Updates tracking-kernel coverage |
tests/unit/test_track_kernels_search_coverage.py |
Updates search-kernel coverage |
tests/unit/test_track_kernels_geom_coverage.py |
Updates geometry-kernel coverage |
tests/unit/test_track_kernels_coverage.py |
Updates public-kernel checks |
tests/unit/test_track_kernels_batch_coverage.py |
Updates batch-kernel coverage |
src/openptv2/tracking_shared.py |
Adds shared linkage utilities |
src/openptv2/plugins/two_phase_tracking.py |
Adds predictive matching and sharing |
src/openptv2/algorithms/track_kernels.py |
Updates kernel re-exports |
src/openptv2/algorithms/track_kernels_transform.py |
Deduplicates transform kernels |
src/openptv2/algorithms/track_kernels_tracking.py |
Removes compatibility shim |
src/openptv2/algorithms/track_kernels_track3d.py |
Adds shared-edge recording |
src/openptv2/algorithms/track_kernels_search.py |
Deduplicates search kernels |
src/openptv2/algorithms/track_kernels_position.py |
Adds position kernels |
src/openptv2/algorithms/track_kernels_position.pxd |
Adds position declarations |
src/openptv2/algorithms/track_kernels_pixel.py |
Updates pixel kernels |
src/openptv2/algorithms/track_kernels_pixel.pxd |
Adds pixel declarations |
src/openptv2/algorithms/track_kernels_geom.py |
Deduplicates geometry kernels |
src/openptv2/algorithms/track_kernels_corr.py |
Updates correlation kernels |
src/openptv2/algorithms/track_kernels_batch.py |
Updates batch kernels |
setup.py |
Updates the Cython module list |
scripts/proto_shared_validate.py |
Adds synthetic validation |
scripts/proto_long_validate.py |
Adds long-window validation |
docs/two-phase-tracking.md |
Documents Two-Phase usage |
docs/trackers.md |
Documents tracker behavior |
docs/plans/2026-09-20-tracking-kernels-dedup.md |
Records kernel deduplication plans |
docs/index.md |
Adds documentation links |
.github/workflows/cibuildwheel.yml |
Updates the wheel import smoke test |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| @@ -58,7 +58,6 @@ | |||
| "track_kernels_position", # 3D ray tracing + position reconstruction | |||
| "track_kernels_track3d", # stereo-3D tracking loop | |||
| "track_kernels_corr", # forward/backward tracking loops + CAS | |||
Comment on lines
+101
to
+102
| if r == -1 and len(holders) >= 2: | ||
| uniq += 2 |
| self.cfg.leaf_weight, | ||
| cost_mode=cost_mode, | ||
| allow_shared=self.cfg.allow_shared, | ||
| share_tol=self.cfg.share_tol, |
Comment on lines
+407
to
+417
| if (int(ai), int(det)) in got_shared and \ | ||
| tr.get("shared_streak", 0) < self.cfg.max_shared: | ||
| # Shared observation: follow the point, keep own speed. | ||
| tr["pos"] = pts1[det].copy() | ||
| tr["shared_streak"] = tr.get("shared_streak", 0) + 1 | ||
| is_shared = True | ||
| else: | ||
| tr["vel"] = (pts1[det] - old_pos) / (gap * self.cfg.dt) | ||
| tr["pos"] = pts1[det].copy() | ||
| tr["shared_streak"] = 0 | ||
| is_shared = False |
Comment on lines
+77
to
+86
| pi = int(prev_k[i]) | ||
| if pi < 0 or k - 1 not in frames: | ||
| continue | ||
| _, _, xyz_p = frames[k - 1] | ||
| if pi >= len(xyz_p): | ||
| continue | ||
| v = xyz_k[i] - xyz_p[pi] | ||
| if np.linalg.norm(xyz_n[j] - (xyz_k[i] + v)) < tol: | ||
| claimants.append((i, float( | ||
| np.linalg.norm(xyz_n[j] - (xyz_k[i] + v))))) |
Comment on lines
+105
to
+109
| ### Two-Phase (`two_phase`) — the hybrid | ||
| 3D search lists candidates, per-camera image distances rank them, Hungarian | ||
| per connected group decides. No motion model — immune to bad guesses, but | ||
| fails once motion outruns particle spacing. Reported +74% multi-frame | ||
| trajectories over the default on a poorly-conditioned aorta dataset. |
Comment on lines
287
to
290
| frame_leaves: list[np.ndarray] | None = None, | ||
| project_fn=None, | ||
| return_chains: bool = False, | ||
| ) -> list[tuple[int, int, int, int]]: |
Owner
Author
|
Superseded by #39, which now contains everything from this prototype: the two-phase plugin and docs were already identical on both branches (shared tree) and merged cleanly; tracking_shared.py, the Fast3D Level-1 sharing hunk (with the claim-cost alloc hoisted so the disabled path is allocation-identical), and both proto validators were merged into the #39 branch (merge commit 76fce9b) and verified there (unit suite green, proto validators run, wp1/S14c/S15 numbers reproduced). Closing to keep a single review thread; nothing was dropped -- the relink-consumer wiring remains follow-up work as noted in #39. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Occlusion lesson: during an occlusion tracks may hold the same point, but each track's speed comes only from its own points.
NOTE: this branch builds on e722951 plus the uncommitted two-phase refactor work (previously dirty in the working tree, now included here) -- the two_phase_tracking.py diff contains that base work on top of which the prototype sits.
What
eturn_chains), giant-component Hungarian cap (production density percolates).
Measured (synthetic 6-particle crossings + N=80 stress)
Flags
All prototype flags default off. 4BE untouched. Per-tracker owners review per tracker before any Cython conversion.