[dexhunter]

TL;DR

• Changes the per-mask loop in SamGeo3.save_masks so it skips work it does not need: boolean masks are used directly, and pixels are counted with np.count_nonzero only when a size filter is active.
• Optimized: SamGeo3.save_masks (samgeo/samgeo3.py), on a workload based on tests/test_samgeo3.py::test_save_masks_promotes_dtype_so_scores_stay_aligned (300 boolean masks of shape (2, 300)) plus edge cases.
• Metric: wall time of the workload relative to the unpatched tree. Each run uses 9 interleaved baseline/candidate pairs on an isolated CPU core and reports the median of the paired ratios. The figures below are the median of 3 such runs.
• Before -> after: 279.1 ms -> 83.5 ms per sample (ratio 0.300, about 3.3x faster). The 3 runs gave ratios of 0.293, 0.300 and 0.311.
• Tested: pytest tests/test_samgeo3.py tests/test_api.py agent-harness/cli_anything/samgeo/tests/test_core.py agent-harness/cli_anything/samgeo/tests/test_full_e2e.py gives 100 passed, the same as on main. The output of every benchmark case matched the unpatched code exactly.

Problem

save_masks loops over every mask. On each pass it creates a new boolean array with mask_np > 0, even when the mask is already boolean. It also always computes the mask size with np.sum(mask_bool). That size is only used for the min_size / max_size filter, which is off by default. In addition, np.sum on a boolean array is much slower than np.count_nonzero. With hundreds of small masks, this per-mask overhead takes up most of the call's run time.

Change

• Reuse mask_np as mask_bool when its dtype is already bool. The array is only read, never written.
• Count pixels only when min_size > 0 or max_size is set, and count them with np.count_nonzero. With min_size <= 0 and no max_size, the old checks could never drop a mask, so the result is the same.
• Use enumerate(self.masks) instead of updating mask_index by hand on each branch.

Nothing else changes: return values, self.objects, self.mask_scores (including key order), dtype promotion and its warning, error messages and the arguments passed to array_to_image are all the same as before.

Benchmark

Benchmark method:

• The benchmark calls the real SamGeo3.save_masks without running the model. common.array_to_image is recorded instead of writing files to disk.
• Cases:
  • 300 boolean masks of shape (2, 300), saved with uint8, int8 and float32 (the last one raises an error), mirroring the repo test.
  • Shuffled masks and scores.
  • Size filters.
  • Binary output (unique=False).
  • Tensor and numpy masks, and masks with singleton axes.
  • Score maps, missing or invalid scores, and empty inputs.
• Each sample runs the case set 18 times. Each run alternates baseline and candidate in 9 pairs (AB/BA) on an isolated core.
• A separate process runs the unpatched code to produce the expected outputs, and both trees must match them exactly.

run	baseline (ms)	patched (ms)	ratio
1	284.4	83.4	0.293
2	279.1	83.5	0.300
3	276.4	86.1	0.311

Run-to-run noise between two unpatched copies (A/A) was within 2.5%.

Tests

• pytest --import-mode=importlib tests/test_samgeo3.py tests/test_api.py agent-harness/cli_anything/samgeo/tests/test_core.py agent-harness/cli_anything/samgeo/tests/test_full_e2e.py: 100 passed (same as on main).
• ruff (v0.16.7, as pinned in pre-commit) reports nothing new in the changed lines.
• No new tests were added. The existing test_save_masks_* tests already cover the dtype promotion and score alignment that this change touches.

AI assistance

This change was found with AI tools. OpenAI Codex (gpt-6.1-sol) generated candidate edits, and the runs were tracked with Weco Observe. A Claude-based agent picked the smallest candidate, checked its output against the unpatched code, and ran the benchmarks and tests above. Only automated checks were run; no human has reviewed the change yet.

Weco share link: https://dashboard.weco.ai/share/7Q74cmqVKQDGESeD00ahLr4Q10JhVaVy

Summary by CodeRabbit

• Bug Fixes
  • Improved mask exports when minimum or maximum size limits are set, with size checks applied consistently across supported mask formats.
  • Saving masks without size limits no longer performs unnecessary size calculations.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 37abe3ff-b6f1-4e77-851b-3a67697acdaf

Commits

Reviewing files that changed from the base of the PR and between 0d7a4ac and 95e77db.

Files selected for processing: 1

• samgeo/samgeo3.py

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

Walkthrough

SamGeo3.save_masks now preserves each mask’s original index for score lookup. It computes mask pixel counts only when size filtering is active and handles Boolean masks directly.

Changes: Mask saving

Layer / File(s)	Summary
Mask iteration and size filtering samgeo/samgeo3.py	The loop uses enumeration to retain original mask indices for score lookup. When size filtering is active, it counts pixels in Boolean masks directly or thresholds other masks at > 0 before applying the existing minimum and maximum checks.

Priority: Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Suggested reviewers: giswqs

Merge Risk: Minimal · up to 95e77

The optimization preserves saved-mask values and size-filter decisions for supported inputs, with no merge-blocking risk identified.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly identifies the performance optimization to SamGeo3.save_masks and includes the reported speed improvement. It accurately summarizes the main change.
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within survi…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files.

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit checks each mask in line
And keeps its score in proper time
When size filters come into play
It counts the pixels on the way
Then hops along to save the day}

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

[github-actions]

Code review

I reviewed the diff against the full save_masks method and the related tests (test_save_masks_records_mask_scores, test_save_masks_promotes_dtype_so_scores_stay_aligned).

Bugs: None found. Traced the two behavioral changes carefully:

• mask_bool = mask_np if mask_np.dtype == np.bool_ else mask_np > 0 — mask_bool is only ever used for read-only boolean indexing (mask_array[mask_bool] = ..., np.count_nonzero(mask_bool)), so aliasing the already-boolean array instead of copying it is safe; no mutation path exists that could corrupt the original mask. Confidence: high.
• filter_by_size = min_size > 0 or max_size is not None computed once, skipping the per-mask np.count_nonzero call when inactive — equivalent to the old per-mask mask_size < min_size / mask_size > max_size checks for every value of min_size (including 0 and negative), since a pixel count is never negative and max_size=None always skipped the second check anyway. Confidence: high.
• Switching from a hand-incremented mask_index to enumerate(self.masks) preserves the same index value in every branch (continue vs. fall-through), so self._score_at(mask_index) still receives the same argument as before. Confidence: high.

Security: None applicable — purely numeric array manipulation on in-memory data, no new I/O, parsing, or external input handling. Confidence: high.

Performance: The stated optimization is sound: avoiding the boolean copy for already-boolean masks and replacing np.sum with np.count_nonzero (and skipping it entirely when no size filter is active) are both legitimate, well-known speedups, consistent with the reported ~3.3x improvement. Confidence: high.

Quality: Code is clean, comments accurately describe the new behavior, and no dead code or unused variables were introduced. Confidence: high.

CLAUDE.md: No CLAUDE.md file exists in this repository, so no project-specific guidelines apply.

No findings to flag; this is a correct, well-tested, narrowly-scoped optimization.