Skip to content

driver: construct real CameraFrame instances instead of plain tuples - #148

Open
amtellezfernandez wants to merge 1 commit into
NVlabs:mainfrom
amtellezfernandez:pr/cameraframe-type-mismatch
Open

driver: construct real CameraFrame instances instead of plain tuples#148
amtellezfernandez wants to merge 1 commit into
NVlabs:mainfrom
amtellezfernandez:pr/cameraframe-type-mismatch

Conversation

@amtellezfernandez

@amtellezfernandez amtellezfernandez commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

models/base.py declares what a model receives:

class CameraFrame(NamedTuple):
    timestamp_us: int
    image: np.ndarray | torch.Tensor

CameraImages = dict[str, list[CameraFrame]]

EgoDriverService._prepare_camera_images is the only place those values get built, and it emits plain tuples. CameraFrame isn't referenced anywhere else in main.py outside a docstring:

camera_images[cam_id] = [(e.timestamp_us, e.image) for e in entries]

So code written against the declared interface gets an object that doesn't satisfy it:

frame = camera_images["front"][-1]
isinstance(frame, CameraFrame)   # False
frame.timestamp_us               # AttributeError: 'tuple' object has no attribute 'timestamp_us'

This patch makes the emitted objects satisfy the declared type.

No in-tree behaviour changes. Every built-in model and the transfuser plugin do for ts, img in frames, and CameraFrame is a NamedTuple, so positional unpacking keeps working. What changes is that isinstance(frame, CameraFrame) becomes True, as the annotation already claims.

The mismatch has also needed compatibility handling downstream: AlpaBridge registers through the alpasim.models entry-point group and currently accepts both representations (1, 2).

Four lines at one site. The other way to resolve it is to weaken the annotation to plain tuples, which drops a documented guarantee.

CameraImages is typed as dict[str, list[CameraFrame]], but _prepare_camera_images builds plain positional tuples, so every PredictionInput.camera_images value a model receives is a bare tuple at runtime, not a real CameraFrame - attribute access (frame.timestamp_us, frame.image) raises AttributeError. Construct real CameraFrame instances; since NamedTuple is a tuple subclass, this is purely additive for existing positional-unpacking consumers.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@copy-pr-bot

copy-pr-bot Bot commented Aug 13, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@amtellezfernandez

Copy link
Copy Markdown
Contributor Author

Closing this after a harder look at necessity: no current consumer is actually affected. Every in-tree model unpacks positionally (which NamedTuple preserves), and external policies — including ours, which is how this was originally noticed — have long since added positional fallbacks. Without a live consumer relying on the declared type, this is contract hygiene rather than a needed fix, and it isn't worth review bandwidth right now. Happy to reopen if the declared CameraImages = dict[str, list[CameraFrame]] type is meant to be a supported contract for plugin authors, or if anyone hits the AttributeError in practice.

@amtellezfernandez

amtellezfernandez commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Reopening: the case is stronger stated purely in terms of this repo, and does not depend on any downstream project.

CameraFrame is the declared interface for camera_images, but _prepare_camera_images emits plain (timestamp_us, image) tuples, so a consumer following the declared attribute interface gets isinstance(frame, CameraFrame) == False and frame.timestamp_us raising AttributeError. Nothing in-tree is affected, since every built-in unpacks positionally and NamedTuple preserves that.

Separately, as a downstream data point: the mismatch has required compatibility handling in AlpaBridge, which registers through the alpasim.models entry-point group and currently accommodates both representations (1, 2). Updated the description accordingly.

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