Skip to content

Add ball-table detection#298

Merged
ekiefl merged 1 commit into
mainfrom
ek/ball-table-detection
May 18, 2026
Merged

Add ball-table detection#298
ekiefl merged 1 commit into
mainfrom
ek/ball-table-detection

Conversation

@ekiefl
Copy link
Copy Markdown
Owner

@ekiefl ekiefl commented May 18, 2026

Summary

The previous PR added the ball-table resolver. This PR completes the
detector→resolver chain by adding a BallTableDetection strategy that emits
BALL_TABLE events when an airborne ball is on a trajectory to intersect the
table plane.

Nothing in main currently produces airborne balls (no resolver writes
const.airborne from a non-airborne state), so the chain is wired but
inert. 2D simulation output is byte-identical to before.

This PR also revisits and simplifies the Dim-validation story around
ball-table that was introduced with the resolver, and patches a latent
NotImplementedError in the transition cache.

What's in this PR

Ball-table detection

New pooltool/evolution/event_based/detect/ball_table.py — mirrors
ball_pocket.py structurally. Single-ball iteration with a 1-tuple cache key
(ball.id,). The cheap state-guard inside ball_table_collision_time returns
np.inf for non-airborne balls, so the strategy is a fast no-op in 2D.

New physics helper in pooltool/physics/motion/solve.py:

@jit(nopython=True, cache=const.use_numba_cache)
def ball_table_collision_time(rvw, s, g, R) -> float:
    if s != const.airborne:
        return np.inf
    return get_airborne_time(rvw=rvw, R=R, g=g)

Just wraps get_airborne_time (already in physics/utils.py from the
airborne-primitives PR) with an airborne-state guard. Lives in
physics.motion.solve alongside the other *_collision_time helpers.

EventDetector wiring (detect/detector.py):

  • Imported BallTableDetection
  • Added ball_table field (after ball_pocket)
  • Wired ball_table.get_next(...) into get_next_event's candidates list
  • Added a tier-3 branch for EventType.BALL_TABLE in _get_event_priority,
    mirroring the cushion-collision semantics. Carries a TODO comment noting
    the choice is provisional — BALL_TABLE-vs-other ties only become real once
    3D activation lands and airborne balls actually arise.

Re-exports in detect/__init__.py: BallTableDetection,
BallTableDetectionStrategy.

INCLUDED_EVENTS in event_based/config.py: added EventType.BALL_TABLE.
Harmless in 2D (detector never emits a finite-time event); correct for future
3D activation where _SimulationState.include would otherwise filter out
BALL_TABLE at resolution time.

CollisionCache.times type widened from
dict[EventType, dict[tuple[str, str], float]] to
dict[EventType, dict[tuple[str, ...], float]]. Ball-table uses a 1-tuple
key (ball.id,) since the event involves a single ball, not a pair. Matches
the 3d branch's precedent for the same reason.

Dim validation simplification

The ball-table resolver PR introduced Dim.THREE + DORMANT_IN_2D to let
the validator accept a "3D-only" ball-table strategy in a 2D engine. On
reflection, that approach paid lip service to the Dim contract: in 2D the
tag was skipped (irrelevant), and in 3D the tag only ruled out a Dim.TWO
ball-table strategy, which is a nonsensical configuration nobody would
write. The Dim taxonomy genuinely doesn't apply to ball-table — the slot
only conceptually exists in 3D.

Reframed this PR:

  • Ball-table strategies don't carry a dim attribute at all. Removed
    from both protocols (BallTableCollisionStrategy,
    BallTableDetectionStrategy) and both concrete resolver classes plus
    the concrete detector.
  • _validate_dimensionality symmetric skip: was if not self.is_3d and field.name in DORMANT_IN_2D: continue (asymmetric); now if field.name in SKIP_DIMENSION: continue (applies in both modes)

Dim docstring + doc cleanup

The Dim enum docstring previously framed Dim.BOTH as "behaves identically
in either mode," which is a stronger claim than the validator actually
requires. A Dim.BOTH strategy is allowed to take different code paths
depending on its input (e.g. branch on state == const.airborne) as long as
neither path is incorrect for the mode it runs under. Updated the docstring
to frame BOTH around safety rather than behavioral equivalence, and
matched the wording in docs/resources/custom_physics.md. As you can see,
still sort of figuring this all out.

Side cleanup: import pooltool as pt

Removed the pt alias from three real-code files where it was used to
access pooltool symbols:

  • tests/evolution/event_based/test_introspection.py
  • tests/physics/resolve/ball_ball/test_frictional_mathavan.py
  • pooltool/ani/constants.py

Docstring >>> examples elsewhere that use pt.* are deliberately
unchanged — they show users the external import idiom, not the project's
internal convention.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 18, 2026

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6e0359a2-a330-4fc5-a19d-dbd720eb3349

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ek/ball-table-detection

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 18, 2026

Codecov Report

❌ Patch coverage is 82.22222% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.87%. Comparing base (a18b4f8) to head (4d86203).

Files with missing lines Patch % Lines
pooltool/evolution/event_based/detect/detector.py 37.50% 5 Missing ⚠️
pooltool/physics/motion/solve.py 50.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #298      +/-   ##
==========================================
+ Coverage   47.81%   47.87%   +0.06%     
==========================================
  Files         157      158       +1     
  Lines       10631    10658      +27     
==========================================
+ Hits         5083     5103      +20     
- Misses       5548     5555       +7     
Flag Coverage Δ
service 47.87% <82.22%> (+0.06%) ⬆️
service-no-ani 58.40% <80.95%> (+0.06%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ekiefl ekiefl merged commit 80a675f into main May 18, 2026
12 checks passed
@ekiefl ekiefl deleted the ek/ball-table-detection branch May 18, 2026 05:00
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.

1 participant