Skip to content

fix: guard polygon construction in segmentation workers - #161

Open
arjunrajlab wants to merge 1 commit into
masterfrom
claude/cellpose-sam-models-empty-by2g6n
Open

fix: guard polygon construction in segmentation workers#161
arjunrajlab wants to merge 1 commit into
masterfrom
claude/cellpose-sam-models-empty-by2g6n

Conversation

@arjunrajlab

Copy link
Copy Markdown
Collaborator

The bug

A Cellpose-SAM run died with:

HTTP error 400: POST .../upenn_annotation/multiple
{"message": "data.coordinates must contain at least 1 items", "type": "validation"}

Annotations upload as one batch, so a single bad polygon took all 442 cells found in that frame down with it.

399df15 fixed the upload chokepoints for this — but only the code that runs after a shapely geometry exists. Construction itself was still bare, and that's where the cellpose-family workers build their geometry, before anything reaches a chokepoint:

  • Polygon(contour) raises ValueError: A linearring requires at least 4 coordinates for a 1–2 point contour (a one-pixel-wide mask);
  • a contour carrying a NaN/inf coordinate builds fine, then detonates inside .simplify() with GEOSException: Non-finite envelope bounds — which is not a ValueError, so it slips past a naive except ValueError.

Either one aborts the whole run, so one unusable mask out of hundreds still loses every good annotation in the frame. The failing run used Padding: -1 and Smoothing: 0.7, i.e. both transform paths were live.

The fix

New guarded helpers in annotation_utilities.annotation_tools, returning None/[] instead of raising:

Helper Purpose
safe_polygon(coords) Build a Polygon, or None. Rejects too-short contours and non-finite coordinates, and drops a third coordinate column (a 3D ring yields 3-tuples, which break the (x, y) unpacking used to build annotation coordinates).
safe_buffer / safe_simplify None-tolerant, GEOS-error-tolerant padding and smoothing.
clean_polygon_coords(coords, padding, smoothing) The whole pipeline: contour → annotation-ready rings, padding then smoothing (the order the cellpose family has always used).

geometry_to_polygon_coords also now rejects a NaN area (every NaN comparison is False, so area <= 0 let it through) and survives GEOS errors raised by is_valid/area.

Sweep

Every worker that builds a polygon from model output had the same latent crash:

  • cellposesam, cellpose, condensatenetrun_model post-processing now goes through clean_polygon_coords. Behavior is unchanged for healthy outlines (same buffer-then-simplify order, preserve_topology=True as before).
  • sam2_video, sam2_propagate, sam/sam2 automatic-mask-generator, sam/sam2 few-shot — guarded safe_* construction, preserving the skip-unusable behavior those functions already had for their no-contours case.
  • sam2_automatic_mask_generator also indexed contours[0] without checking for an empty contour list — IndexError on a mask with nothing traceable.
  • sam/sam2 few-shot computed training-area stats from user annotations, where one degenerate annotation aborted the run before inference started.
  • worker_client — the upload chokepoint now uses safe_polygon, so it catches GEOS errors too (its except (ValueError, TypeError) did not); helpers are re-exported for workers.

Removed the now-unused shapely.geometry.Polygon imports (duplicated in several of these files).

Deliberately not covered: the cellpose_train / cellposesam_train region-polygon reads, where a degenerate user annotation can still abort a training run — a different surface (user input, not model output) and out of scope here. It's recorded in the hardening skill catalog, which gains the construction-time half of this failure mode.

Tests (TDD: red → green)

workers/annotations/cellposesam/tests/test_run_model.py is new — this path had no coverage. deeptile/cellpose are stubbed so it runs natively in the light venv, no GPU image needed. 6 of its 10 tests reproduced the crash before the fix; the other 4 pin existing post-processing behavior (negative padding erosion, MultiPolygon splitting, pass-through, smoothing actually simplifying).

annotation_utilities:  35 passed   (+17 new)
worker_client:         15 passed   (+3 new)
cellposesam:           23 passed   (+10 new)

Docker worker tests were not run — no Docker in this environment. The pre-existing few-shot worker test suites need the in-image annotation_client and fail to collect natively both before and after this change.

CELLPOSESAM.md gains a note that degenerate outlines are dropped and that a pinched object yields one annotation per piece, so a frame's annotation count need not equal its mask count.


Generated by Claude Code

The reported Cellpose-SAM run died with HTTP 400 `data.coordinates must
contain at least 1 items`, losing all 442 annotations found in the frame.
399df15 fixed the upload chokepoints, but only the code *after* a shapely
geometry exists. Construction itself was still unguarded, and that is where
the cellpose-family workers actually build their geometry -- before anything
reaches the chokepoint:

- `Polygon(contour)` raises `ValueError: A linearring requires at least 4
  coordinates` for a 1-2 point contour (a one-pixel-wide mask);
- a contour carrying a NaN/inf coordinate builds fine, then detonates inside
  `.simplify()` with `GEOSException: Non-finite envelope bounds` -- not a
  ValueError, so it slips past a naive `except ValueError`.

Either one aborts the whole run, so one unusable mask out of hundreds still
loses every good annotation in the frame.

Add guarded helpers to annotation_utilities -- `safe_polygon`, `safe_buffer`,
`safe_simplify`, and `clean_polygon_coords` (contour -> annotation-ready rings,
applying padding then smoothing) -- which return None/[] instead of raising.
`safe_polygon` also rejects non-finite coordinates up front and drops a third
coordinate column, which would otherwise yield 3-tuples and break the (x, y)
unpacking used to build annotation coordinates. `geometry_to_polygon_coords`
now also rejects a NaN area (every NaN comparison is False, so `area <= 0`
let it through) and survives GEOS errors from `is_valid`/`area`.

Sweep of every worker that builds a polygon from model output:
- cellposesam, cellpose, condensatenet: `run_model` post-processing now runs
  through `clean_polygon_coords`;
- sam2_video, sam2_propagate, sam/sam2 automatic-mask-generator, sam/sam2
  few-shot: guarded `safe_*` construction, keeping the skip-unusable behavior
  those functions already had for the no-contours case;
- sam2_automatic_mask_generator also indexed `contours[0]` without checking
  for an empty contour list (IndexError on a mask with nothing traceable);
- the few-shot workers' training-area stats would abort a run on a degenerate
  *user* annotation;
- worker_client's upload chokepoint now uses `safe_polygon`, so it catches GEOS
  errors too, and the helpers are re-exported for workers.

Not covered: the `cellpose_train`/`cellposesam_train` region-polygon reads,
where a degenerate user annotation can still abort a training run. Noted in
the hardening skill catalog, which gains the construction-time half of this
failure mode.

Tests (red -> green): 10 new for cellposesam's `run_model`, which had no
coverage of this path at all (deeptile/cellpose stubbed, so it runs natively);
17 new in annotation_utilities; 3 new in worker_client.

  annotation_utilities: 35 passed
  worker_client:        15 passed
  cellposesam:          23 passed

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012j37xD35rbyXcHmLPJ6Knq
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.

2 participants