Repository navigation
feat(evals): add four deterministic VQA families - #4541
moshiii-wei wants to merge 4 commits into
Conversation
Extend VQA-Gen with vertical_direction, presence_and, object_color, and camera_bearing so more spatial and attribute questions can be proposed by the author and answered from detector, color, and LiDAR primitives.
7e19576 to
a0d8dc5
Compare
github-actions
Bot
added
the
first-time-contributor
a0d8dc5 to
da23e93
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
go2 replay realtime (arm64)
Details
| Benchmark suite | Current: 95d3f89 | Previous: 19e9c0a | Ratio |
|---|---|---|---|
peak memory |
1587.887 MB |
1569.781 MB |
1.01 |
peak threads |
350 threads |
346 threads |
1.01 |
network (transport) |
2724.831 MB |
2724.693 MB |
1.00 |
disk write |
1.535 MB |
1.805 MB |
0.85 |
instructions |
115.369 G |
115.585 G |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
❌ 1 Tests Failed:
View the full list of 1 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| valid = (sat >= 40) & (val >= 40) | ||
| if int(valid.sum()) < _MIN_VALID_PIXELS: | ||
| raise InsufficientEvidenceError( | ||
| "object color crop lacks enough saturated, well-lit pixels" | ||
| ) |
There was a problem hiding this comment.
dominant_color_in_bbox removes every pixel that could become white, black, or gray before counting colors. Solid neutral-colored objects are rejected, while a mostly neutral object can receive the color of a small saturated patch. A crop containing 95% white and 5% blue was accepted and saved with blue as its expected answer, corrupting evaluation ground truth. This needs fixing before merging.
Count neutral pixels too, and measure the winning color against the full usable crop.
Artifacts
Complete executed neutral-color validation script
- The authored script exercises five synthetic images through the real primitive, family dispatch, and dataset persistence paths, allowing both observed runs to be reproduced.
Neutral-color results with the filter disabled in memory
- The diagnostic control executes the same five images with only the filter disabled and reads back saved labels, showing all neutrals classified and the predominantly white crop labeled white.
Neutral-color results on unmodified PR head
- The unmodified head executes the same five images and reads back saved labels, confirming three neutral rejections and an incorrect blue label for the predominantly white crop.
Full executed script and exact production source excerpts
- A command captures the complete test source, numbered filter and label-persistence excerpts, head revision, and tracked diff, showing the precise tested logic and no production edits.
| if hue < 150: | ||
| return COLOR_CHOICES.index("purple") | ||
| return COLOR_CHOICES.index("brown") |
There was a problem hiding this comment.
_hue_to_bucket treats hues 150–169 as brown, so vivid pink and magenta pixels get labeled brown. Ordinary brown pixels, such as RGB (128, 64, 0), fall in the orange hue range instead. Actual image crops reproduced both errors through answer_question, where these labels become incorrect expected answers. This needs fixing before merging.
Use brightness along with hue to distinguish brown from orange, map pink or magenta to a suitable color group or reject unsupported colors, and add regression tests for both cases.
Artifacts
Complete executable color reproduction script
- The uploaded script constructs real RGB and BGR Image objects, executes the primitive and family dispatcher, and checks the cited hue interval, making the reproduction rerunnable.
Full source of the executed reproduction script
- Captured cat output contains the entire executed script with its command, working directory, and exit code, preserving the full test source.
Base revision run with the color API absent
- The same script ran against base 1e7b491 and exited 3 because the color module does not exist, establishing the honest pre-PR baseline.
PR revision run showing incorrect brown and pink labels
- The script ran against the PR head and exited 1 after six failed color expectations, while controls passed and all 20 cited hues returned brown, confirming candidate1.
Exact source lines and unchanged production files
- Executed git and numbered-source commands captured the faulty bucket branches and answer propagation with no tracked diff, linking the observed failures to unchanged PR code.
| winner = int(counts.argmax()) | ||
| fraction = float(counts[winner] / counts.sum()) | ||
| if fraction < _MIN_BUCKET_FRACTION: |
There was a problem hiding this comment.
counts.argmax() accepts a tied winner because _MIN_BUCKET_FRACTION is only 0.4. A crop split equally between red and blue passes with a fraction of 0.5 and saves red solely because it comes first in COLOR_CHOICES. Dataset generation reproduced this arbitrary expected answer; a model choosing the equally common blue would be marked wrong. This needs fixing before merging.
Reject ties and require a clear lead over the runner-up before saving a single dominant color.
Artifacts
Complete tied-color reproduction script
- The uploaded full source executes the base contract and current primitive, dispatcher, and dataset writer, making the confirmed reproduction repeatable.
Before PR: color proposal rejected
- Executed the exact base revision’s proposal contract with object_color and captured its validation rejection, showing the family did not exist before this PR.
After PR: tied colors saved as red
- Ran equal red/blue pixels through the real primitive and dataset writer and read back the red label, confirming ambiguous evidence becomes saved ground truth.
Exact source for tie selection and label persistence
- Captured numbered source excerpts and git diff --exit-code, locating the unchecked tie and label propagation while confirming no production edits.
| requires_pointcloud=True, | ||
| requires_masks=True, |
There was a problem hiding this comment.
CAMERA_BEARING_FAMILY sets requires_pointcloud=True, but the generator uses that flag to require a range_estimator too. A caller of generate_frames_dataset with a point cloud and mask_estimator, leaving the optional range_estimator=None, never gets camera_bearing offered even though its answer needs no range estimator. This is a non-blocking loss of coverage for programmatic callers; the CLI is unaffected because it already provides one.
Separate the point-cloud requirement from the range-estimator requirement and cover this combination in a generation test.
Artifacts
Executable bearing service reproduction
- The full script executed against both revisions captures offered families and exercises generation plus direct answering, demonstrating the unnecessary range-estimator gate.
- Ran the synthetic service reproduction against the PR base and captured source excerpts and responses, showing that camera bearing had not yet been introduced.
Service output after PR4541 with and without a range estimator
- Ran the same reproduction against PR head and captured generated datasets and direct answers, confirming that bearing is hidden without an estimator it never uses.
| "is clearly in the top, middle, or bottom third." | ||
| ), | ||
| ) | ||
| PRESENCE_AND_FAMILY = FamilySpec( |
There was a problem hiding this comment.
why do we need this when we have presence family. How is asking for 2 objects presence together so different than asking seperately?
There was a problem hiding this comment.
it is an easier version of logic questions like, can you push the chair under the table. then it has to identify a scene where chair and table co-exist
There was a problem hiding this comment.
Keeping it — joint presence in one frame is a lighter stand-in for multi-object “can both exist here” style questions, even though it overlaps single presence.
There was a problem hiding this comment.
@moshiii-wei Please configure your agent to sign messages (e.g. — Codex/Claude) so we know if we're talking to you or your agent.
| sat = hsv[:, :, 1].astype(np.intp) | ||
| val = hsv[:, :, 2].astype(np.intp) | ||
|
|
||
| valid = (sat >= 40) & (val >= 40) |
There was a problem hiding this comment.
this condition eliminates black, white, maybe gray colors. which is wrong. val, sat for these colors are less than 40 in your _hue_to_bucket fucntion below
There was a problem hiding this comment.
will fix thanks
There was a problem hiding this comment.
Fixed — neutral pixels are counted again and can resolve to white / black / gray.
| y0 = max(0, min(y0, image.height - 1)) | ||
| y1 = max(y0 + 1, min(y1, image.height)) | ||
|
|
||
| crop = image.to_rgb().data[y0:y1, x0:x1] |
There was a problem hiding this comment.
for color, pixels inside segmented mask must be considered, and not the bounding box. coz background oixels may dominate
There was a problem hiding this comment.
Fixed — color now uses the EdgeTAM mask (dominant_color_in_mask), not the bbox crop.
| if int(valid.sum()) < _MIN_VALID_PIXELS: | ||
| raise InsufficientEvidenceError("object color crop lacks enough saturated, well-lit pixels") | ||
|
|
||
| buckets = np.array([_hue_to_bucket(int(h), int(s), int(v)) for h, s, v in hsv[valid]]) |
There was a problem hiding this comment.
using vectorized np.select over the hue/sat/val arrays might 10x speed
There was a problem hiding this comment.
Done — bucketing is vectorized with np.select now.
|
|
||
| rgb = np.asarray(crop[:, :, :3], dtype=np.uint8) | ||
| hsv = cv2.cvtColor(rgb, cv2.COLOR_RGB2HSV) | ||
| hsv[:, :, 0].astype(np.intp) |
There was a problem hiding this comment.
this looks odd, youre not assigning
There was a problem hiding this comment.
Good catch — that line dropped the hue = assignment. That path is gone now; color is mask-based and vectorized.
| box = detections.detections[0].bbox | ||
| evidence = dominant_color_in_bbox(image, object_name, box) | ||
| return FamilyAnswer( | ||
| question=f"What is the dominant color of the visible {object_name}?", |
There was a problem hiding this comment.
currently object_name are sometimes referred with color such as red bag , black couch so, are we making sure thats not happening?
There was a problem hiding this comment.
good question need to check
There was a problem hiding this comment.
Fixed — names that already contain a color word (e.g. red bag) are rejected for object_color.
| calibration_source: str = Field(min_length=1) | ||
|
|
||
|
|
||
| def estimate_camera_bearing( |
There was a problem hiding this comment.
i find this same as horizontal_direction family. Its just that [left | middle/ahead | right] these sections differ in widths to left and right. should we drop this?
There was a problem hiding this comment.
Agreed — dropped camera_bearing and kept horizontal_direction.
Address review feedback: mask-based color with neutrals/ties, ban colorful object names, vectorize HSV bucketing, add optional family-balance menu caps, and remove camera_bearing as redundant with horizontal_direction.
8bd69e9 to
871c1a8
Compare
|
@greptile |
| clipped = tuple( | ||
| family | ||
| for family in capable | ||
| if family.name not in shares or counts.get(family.name, 0) / total < shares[family.name] | ||
| ) | ||
| return clipped if clipped else tuple(capable) |
There was a problem hiding this comment.
With family_balance="uniform", families that cannot produce answers can permanently crowd out those that can. After presence, presence_and, and object_count each produce one answer, their shares exceed 1/N. If the remaining families keep rejecting ambiguous detections, only those unsuccessful families stay in the menu.
Their counts remain zero, so the menu never becomes empty and the fallback never runs. Generation on 12 identical frames saved only three cases from the first frame, compared with 36 cases without balancing. Retry the full capable menu when the restricted menu produces no answers.
Artifacts
- The executed standalone reproduction supplies a menu-respecting author and structured detector, reads persisted generation outputs, and asserts both policy outcomes, making the comparison reproducible.
- Generated the 12-frame scene with family_balance=none and captured menus, saved cases, labels, and audits, showing all frames remained productive with 36 total cases.
- Generated the same 12-frame scene with family_balance=uniform and captured menus, saved cases, labels, and audits, showing only the first frame produced cases while the remaining menus stayed restricted.
| _COLOR_NAME_PATTERN = re.compile( | ||
| r"(?<![a-z0-9])(" + "|".join(re.escape(name) for name in COLOR_CHOICES) + r")(?![a-z0-9])", | ||
| re.IGNORECASE, | ||
| ) |
There was a problem hiding this comment.
The color-word guard accepts grey sofa because COLOR_CHOICES includes gray but not grey. _answer_object_color then copies that name into the question. Generation saved What is the dominant color of the visible grey sofa? with expected answer gray, while correctly rejecting gray sofa.
This is a non-blocking evaluation-quality issue: the saved question supplies an answer hint instead of requiring color recognition. Include grey in the rejected color words and add a regression test.
Artifacts
- The complete authored harness executes revision-specific VQA code with synthetic evidence and spelling controls, making the result reproducible.
- Ran the harness against the PR base and captured schema rejection and the absence of a published dataset, establishing that this family is new in the PR.
- Ran the harness against the PR head and read back the generated dataset, confirming that grey sofa leaks the gray answer while gray sofa is rejected.
| ), | ||
| family_max_share: list[str] | None = typer.Option( | ||
| None, | ||
| help="Cap accepted share for a family as name=share (repeatable), e.g. presence=0.25", |
There was a problem hiding this comment.
The help describes --family-max-share as a cap on accepted cases, but the option only changes later author menus. The first frame is unrestricted, and every proposal in a frame is answered before counts change. The CLI accepted presence=0.25 and saved a single-frame dataset containing four presence cases and no other cases: a 100% share despite the recorded 25% limit.
This is a non-blocking output-policy mismatch that can make users mistake a preference for a guaranteed dataset limit. Describe it as a best-effort menu preference, including the fallback, unless the generator actually enforces the final share.
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!
This branch has not been deployed
4 participants
What is this feature?
Adds four image- and LiDAR-backed VQA families on top of existing VQA-Gen (
QuestionAuthor→answer_question→cases.jsonl):vertical_direction— top / middle / bottom from a single detection bboxpresence_and— both object categories detected (reject if either is missing)object_color— dominant HSV bucket inside one detection cropcamera_bearing— left / ahead / right from LiDAR points in an EdgeTAM maskNew primitives live under
dimos/evals/vqa/primitives/{color,bearing}.py. The OpenAI author already receives family metadata viaAvailable families: …JSON; no separate prompt change.Why do we need this?
Improves coverage toward related-work question types (spatial layout, multi-object presence, color attributes, camera-relative bearing) while keeping deterministic GT. Supports the goal of more families backed by auditable primitives rather than LLM answers.
How to Test
cd dimos uv run pytest dimos/evals/vqa -qOptional smoke on a calibrated recording (needs
OPENAI_API_KEYfor the author):Inspect
audit/run.jsonfor new family names and samplecases.jsonlquestions.Which issue(s) does this PR close?
Fixes
Checklist
Made with Cursor