Restore stitched TIFF illumination correction worker - #176
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c726d2d3ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return { | ||
| "P2_n_objects": int(keep.sum()), | ||
| "P2_spearman": float(spearmanr(raw_sum[keep], corrected_sum[keep]).statistic), | ||
| "P2_applicable": True, |
There was a problem hiding this comment.
When a plane contains at least ten measurable objects with identical summed intensities, spearmanr(raw_sum, corrected_sum) returns NaN, but this branch still reports P2_applicable=True. check_guardrails() consequently treats the non-finite result as a hard violation, so every candidate—including the identity model—can be rejected and an otherwise valid correction job fails. Check that both rank inputs have variation, and mark P2 inapplicable when the correlation is undefined.
Useful? React with 👍 / 👎.
|
Addressed in Branch-wide audit found no other Spearman consumers, uncertainty-expanded tie calculations, or unscoped progress callbacks. The worker Docker suite passes all 42 tests. @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b642158e2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if np.ptp(raw_values) <= np.finfo(float).eps or np.ptp( | ||
| corrected_values | ||
| ) <= np.finfo(float).eps: |
There was a problem hiding this comment.
Unlike the previously reported constant-raw case, when raw object sums vary but a correction collapses all corrected sums to one value, the new corrected_values arm marks P2 inapplicable. check_guardrails() then skips the Spearman guardrail, allowing a candidate that erased every object-intensity distinction to remain valid and potentially win; only constant raw values should make preservation unassessable, while constant corrected values with variable raw input should be a violation.
Useful? React with 👍 / 👎.
| consistent = _consistently_improves( | ||
| selected, identity, use_spot_uniformity | ||
| ) | ||
| if aggregate_improvement <= base_margin or not consistent: |
There was a problem hiding this comment.
When multiple Pareto corrections are within the 5% tie margin, the simpler candidate is selected before paired consistency is checked. If that simpler candidate regresses one held-out plane, this branch immediately falls back to identity even when another tied candidate improves identity strongly and consistently on every plane, so Automatic mode can discard a correction that satisfies its documented displacement rule; filter candidates through the identity gate before tie-breaking or retry the remaining tied candidates.
Useful? React with 👍 / 👎.
|
Addressed both new findings in
I also generalized the audit across the branch and added regressions for both tie-stage and Pareto-stage masking. All 45 Docker tests pass. @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3a1ad5647
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| corrected_sum = ndimage.sum_labels( | ||
| corrected.astype(np.float64), labels, indices | ||
| ) | ||
| keep = (raw_sum > 0) & (corrected_sum > 0) |
There was a problem hiding this comment.
When a correction makes enough previously positive object sums zero or negative, this filter removes those objects; if fewer than ten remain, the next branch marks P2 unavailable. On a large mosaic, erasing a small object can remain below P5's 1e-4 pixel-fraction threshold, so the candidate can pass every hard guardrail despite destroying biological signal. The new d3a1ad5 constant-collapse check does not cover this case because this early return bypasses it; keep raw-positive objects in the rank calculation and treat nonpositive corrected sums as a preservation failure. This also contradicts the worker documentation's claim that only a constant raw baseline makes this guardrail unavailable.
AGENTS.md reference: AGENTS.md:L49-L57
Useful? React with 👍 / 👎.
|
Addressed the P1 finding in P2 now defines its preservation cohort from raw-positive objects and no longer filters objects based on corrected positivity. Zero or negative corrected object sums are retained and reported through a zero-tolerance erased-object guardrail, including when too few objects exist to calculate Spearman rank. I searched all worker preservation metrics for the same corrected-outcome survivor-bias pattern. This was the only instance; it is fixed, with regressions for zero, negative, and sub-10-object cases. All 48 Docker tests pass, and worker provenance is now @codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
pyvips/libvips runtime required by final TIFF conversionTDD and review
pyvipsCFFI initialization failure (red), then pinnedpyvips==3.1.1.8.18.2, added build-time/runtime ABI assertions, and serialized libvips concurrency (green)1.0.3Validation
./build_workers.sh --build-and-run-tests stitched_illumination_correction— 48 passed, including real BaSiC fits, the production pyvips/libvips runtime check, candidate-masking regressions, and erased-object preservation regressionsgit diff --check origin/master...HEADdocker compose config --quietisGPUWorker=falseandworkerVersion=1.0.3.agents/skills/nimbus-run-worker/scripts/run_worker.pyon dataset view6a6de7c89be9076cdae3ef17uint16Algorithm clarification
The stitched worker's Folded log-gradient method uses an FFT/Fourier-domain periodic Poisson solve. It is not the overlap-DCT method. The separate raw-ND2 Stitch Refinement + Illumination Correction worker uses order-5 overlap-DCT because it still has access to independent raw-tile overlaps.
Source
The recovered implementation is based on
codex/findings-based-illumination-correction(155a53a), renamed and isolated so it can coexist with the raw-ND2 stitch-refinement worker.