Estimator followups - #7
Merged
Merged
Conversation
_intrinsics_after_crop returned cy = 200, the half-height of a 600x400 Street View frame, for every input. ImageRequest.size is configurable and the API serves up to 640x640, so any other capture height placed the horizon on the wrong row -- 120 rows out for a 640-tall frame -- and the horizon is the reference the width band is measured against, so the error propagates into every metre reported. cy now comes from the frame's own height, with crop_bottom_px for a caller that trimmed the logo strip: cropping the bottom does not move the rows above it, so the principal point must still be reported in the original frame's coordinates. For the 600x400 the pipeline actually fetches, 400/2 is the same 200 as before -- the 1000-scenario compute_width golden file is unchanged, which is what confirms this is a generalisation and not a change of behaviour. Two things that shared this concern came along: read_rgb's crop was `rgb[:-bar]` with no guard, and the default bar height is 0, so `rgb[:-0]` is `rgb[:0]`: enabling SWAI_IMG_AUTO_CROP_GOOGLE_LOGO without also setting a height returned an empty image. The pipeline now reads with crop_bar=False explicitly. It already handles the strip by ignoring rows rather than removing them -- bottom_ignore_px for width, NaN for depth -- so honouring the env flag as well would have taken the strip off three times over and shifted every row index against the depth map.
Back-ends returned a bare 3- or 4-tuple and callers told the two apart with
`len(out)`. Nothing recorded what the positions meant, an arity outside {3, 4}
left every name unbound, and each consumer reimplemented the same normalisation:
the pipeline in two places, the ensemble in a third.
SegmentationOutput is a frozen dataclass with mask/seg_map/seg_info/obstacles,
obstacles defaulting to an empty list because the pipeline iterates it without a
None check. `coerce` still accepts the legacy tuples, so a third-party segmenter
keeps working, and takes the back-end's name so a bad return value points at its
author instead of at the pipeline.
Behaviour is unchanged, checked against real weights rather than inferred: on the
same frame OneFormer still reports 4.28 m with 2 obstacles, Detectron2 3.84 m
with 4, DeepLab 5.00 m with 0, and the oneformer+detectron2 ensemble 4.05 m with
4 -- every figure identical to before the migration.
`extract_obstacles` applied one hardcoded vocabulary, written against ADE20K's 150 classes, to every back-end. Cityscapes has 19 coarse ones, and the mismatch is most of why DeepLab and OneFormer disagree about what sits on a sidewalk: what OneFormer calls `grass` is Cityscapes' `terrain`, which the shared set ignores as ground. `ignore_labels` and `sidewalk_labels` now travel on SegmentationOutput, next to the map they describe, and `extract_obstacles` takes both as arguments. Putting them on the output rather than on the segmenter matters twice over: the CLI wraps every back-end in AliasSegmenter, which would not have forwarded an attribute, and the ensemble passes through one member's map, so it can copy that member's vocabulary rather than guess. models/deeplab.py spells out the Cityscapes set, identical in effect to what the shared default already did, but now reviewable and editable without touching shared code. Label matching was `startswith` against the whole string. That is deliberate -- ADE20K labels are synonym lists like "building, edifice" -- but it also let a class swallow every longer name sharing its prefix: "skyscraper" was discarded as "sky". Matching now compares the first synonym exactly. Corrects the measurement in docs/reproducibility.md too. The earlier claim of zero obstacles came from three frames; over ten, `pole` and `vegetation` do fire, in one and three frames. The gap with OneFormer is real but narrower than stated, and the ignored-but-touching classes are now listed so the `car` question can be settled against ground truth. Finally, zero obstacles printed a bare `nan` median beside rating III, which reads identically whether the sidewalk is clear or the detector found nothing because it could not. The CLI now says which. Behaviour verified unchanged against real weights: DeepLab still 5.00 m with 0 obstacles, OneFormer 4.28 m with 2, on the same frame as before.
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
1 participant
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.
No description provided.