Skip to content

Add per-frame RHEED segmentation masks to the SDK - #96

Closed
chris-price19 wants to merge 2 commits into
mainfrom
feat/masks-to-rheed-result-bundle
Closed

Add per-frame RHEED segmentation masks to the SDK#96
chris-price19 wants to merge 2 commits into
mainfrom
feat/masks-to-rheed-result-bundle

Conversation

@chris-price19

Copy link
Copy Markdown
Contributor

What

Adds SDK support for the backend's per-frame RHEED segmentation-mask endpoint (GET /rheed/images/{data_id}/frame_masks) and bundles the masks into the RHEED timeseries.

New public surface

  • Client.get_frame_masks(data_id, *, from_frame=0, to_frame=None, decode=False) — fetches per-frame COCO-RLE masks for a processed RHEED video.
    • to_frame=None fetches the whole video via a clamp sentinel (no preliminary frame-count lookup).
    • decode=False returns raw rows; decode=True returns {frame_number: (H, W) uint8 ndarray}.
    • A 404 ("no frame-mask artifact") maps to an empty []/{} rather than raising.
    • Client-side validation (from >= 0, to >= 0, to >= from) fails fast with a clear ValueError instead of a server 422.
  • Client.get_rheed_timeseries(..., include_masks=False) — when True, fetches the masks and joins them onto the timeseries DataFrame's Frame Number axis as mask_rle / mask_height / mask_width columns. Skips the fetch when the series is empty.
  • decode_mask_rle(mask_rle, height, width) — reusable COCO-RLE → binary-mask helper, exported from atomscale.results.

Internals

  • RHEEDProvider.attach_frame_masks(df, mask_rows) — left-joins mask rows onto the timeseries by absolute frame number. Handles sparse coverage (rotating videos → NA for frames with no mask), missing artifact (columns present, all-NA), no-frame-axis passthrough, and re-attach (drops existing mask columns so the join never suffixes duplicates).
  • Refactored the existing inline mask decode in _get_rheed_image_result to use the shared decode_mask_rle, so the single-frame /mask and per-frame /frame_masks paths decode through one implementation.

Why

Per-frame masks let callers overlay the segmented diffraction pattern on any frame of the processed video, keyed identically to the RHEED timeseries Frame Number axis. Bundling into get_rheed_timeseries(include_masks=True) makes the RLE first-class alongside the per-frame features so everything is keyed by frame in one DataFrame.

Usage

# Standalone, decoded masks keyed by frame
masks = client.get_frame_masks(video_id, decode=True)   # {frame_number: HxW uint8}

# Bundled into the timeseries DataFrame
df = client.get_rheed_timeseries(video_id, include_masks=True)
row = df.iloc[0]
mask = decode_mask_rle(row["mask_rle"], row["mask_height"], row["mask_width"])

Tests

Adds offline unit tests (monkeypatched _get, no network):

  • test_rheed_image.py: raw rows, whole-video sentinel, explicit ranges, RLE decode round-trip, 404→empty, invalid-range rejection, and the missing-mask regression still passes after the decode refactor.
  • test_rheed_timeseries.py: attach_frame_masks sparse/empty/no-axis/re-attach cases, the include_masks=True integration path, and a guard that the default makes no /frame_masks call.

All 25 offline tests pass; source is clean under CI-pinned ruff@0.9.4.

Reviewer notes / open choices

  • include_masks fetches the whole video's masks (not scoped to a last_n / elapsed_seconds window). Easy to scope to the returned window if preferred.
  • Not wired into the client.get()RHEEDVideoResult.timeseries_data path (that method has no per-type options); could be added to RHEEDProvider.build_result if we want masks on bulk get.
  • Named get_frame_masks (pairs with the existing get_frame); the source plan sketched get_rheed_frame_masks — trivial rename if you'd rather match it.

🤖 Generated with Claude Code

@greptile-apps

greptile-apps Bot commented Aug 8, 2026

Copy link
Copy Markdown

Greptile Summary

Adds SDK support for retrieving and decoding per-frame RHEED segmentation masks and optionally joining their RLE data into RHEED timeseries results.

  • Exposes get_frame_masks with range validation, raw and decoded result modes, and missing-artifact handling.
  • Extracts and exports a shared COCO-RLE decoding helper.
  • Adds frame-number-based mask attachment and offline coverage for fetching, decoding, sparse joins, and opt-in behavior.

Confidence Score: 4/5

The PR appears safe to merge, with a non-blocking efficiency issue when mask enrichment is combined with a restricted timeseries window.

The new retrieval, decoding, and join paths have targeted tests and no concrete correctness or security failure remains, but windowed timeseries queries currently download all frame masks before discarding out-of-window rows.

Files Needing Attention: src/atomscale/client.py

Important Files Changed

Filename Overview
src/atomscale/client.py Adds the public mask-fetching API and optional timeseries enrichment; restricted timeseries requests still retrieve the complete mask artifact.
src/atomscale/results/rheed_image.py Extracts the existing pycocotools decoding operation into a shared public helper without changing its decoding semantics.
src/atomscale/timeseries/rheed.py Adds sparse left-join support for attaching mask metadata by absolute Frame Number while preserving unmatched timeseries rows.
src/atomscale/results/init.py Exports decode_mask_rle through the atomscale.results public namespace.
tests/test_rheed_image.py Adds offline coverage for range handling, raw and decoded masks, missing artifacts, and RLE round trips.
tests/test_rheed_timeseries.py Covers sparse, empty, axis-free, repeated attachment, opt-in integration, and default no-fetch behavior.

Sequence Diagram

sequenceDiagram
    participant U as SDK caller
    participant C as Client
    participant T as RHEED timeseries endpoint
    participant M as Frame-mask endpoint
    participant P as RHEEDProvider
    U->>C: "get_rheed_timeseries(data_id, include_masks=True)"
    C->>T: "GET /rheed/timeseries/{data_id}/"
    T-->>C: Timeseries rows
    C->>P: to_dataframe(raw)
    P-->>C: DataFrame keyed by Frame Number
    alt DataFrame is non-empty
        C->>M: "GET /rheed/images/{data_id}/frame_masks"
        M-->>C: Sparse COCO-RLE mask rows
        C->>P: attach_frame_masks(df, rows)
        P-->>C: Left-joined DataFrame
    end
    C-->>U: RHEED timeseries DataFrame
Loading

Fix All in Claude Code Fix All in Conductor

Prompt To Fix All With AI
### Issue 1
src/atomscale/client.py:650
**Windowed queries fetch every mask**

With `include_masks=True`, `last_n` and `elapsed_seconds` restrict the timeseries response but the subsequent `get_frame_masks` call still retrieves the complete mask artifact. Most masks are then discarded by the left join, adding avoidable transfer, deserialization, latency, and memory costs for long videos.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "add masks to timeseries fetch" | Re-trigger Greptile

Comment thread src/atomscale/client.py Outdated
ts_df = provider.to_dataframe(raw)
# Skip the mask fetch when the series is empty — there are no frames to key
# masks onto, so the extra request would be wasted.
if include_masks and not ts_df.empty:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Windowed queries fetch every mask

With include_masks=True, last_n and elapsed_seconds restrict the timeseries response but the subsequent get_frame_masks call still retrieves the complete mask artifact. Most masks are then discarded by the left join, adding avoidable transfer, deserialization, latency, and memory costs for long videos.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/atomscale/client.py
Line: 650

Comment:
**Windowed queries fetch every mask**

With `include_masks=True`, `last_n` and `elapsed_seconds` restrict the timeseries response but the subsequent `get_frame_masks` call still retrieves the complete mask artifact. Most masks are then discarded by the left join, adding avoidable transfer, deserialization, latency, and memory costs for long videos.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code Fix in Conductor

With include_masks=True, a last_n / elapsed_seconds query windowed the
timeseries but get_frame_masks still fetched the whole video's mask
artifact, discarding most rows in the left join. Add
RHEEDProvider.frame_number_bounds() and pass from_frame/to_frame derived
from the returned series so only the spanned frames' masks are fetched.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@chris-price19

Copy link
Copy Markdown
Contributor Author

tests are passing locally

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