Repository navigation
Publish ROI total in range and the detector-pixel count of each detector-view ROI - #1360
Merged
Merged
Conversation
This was referenced Oct 8, 2026
… ROI
Requirement (TBL, applies to all ROI-capable detector views): show the
counts summed over an ROI and summed over the spectral axis, and the same
averaged over the ROI's pixels.
Adds roi_counts_{cumulative,current} and
roi_counts_per_pixel_{cumulative,current}, 1-D over `roi`. Both sum over the
active range filter, like the detector image. The pixel count is the number
of screen pixels (display pixels) in the ROI, obtained by running the ROI
extraction on an all-ones image so it follows the same selection as the
spectra.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ange slice Review follow-ups for the per-ROI counts outputs: - Rename roi_counts_* to roi_counts_in_range_* (and the per-pixel variant), and the view titles to "ROI total in range" / "ROI counts per pixel in range". Unlike the ROI spectra next to them, these respect the range filter, matching the existing Total / Total in range pair. - State in the output descriptions that every pixel of the displayed image counts and pixel weighting is not applied. - Select the range filter through one helper, slice_spectral_range, used by the detector image, the total in range, and the per-ROI counts, so they cannot pick different bins. - Derive the expected message count in the service tests from the declared outputs. Test the range filter end to end, the window time coords of the new outputs, an ROI without pixels, and the fake backend's per-ROI scalars. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Counts per pixel in an ROI now divide by the number of detector pixels inside the ROI, summed from the pixel weights, instead of by the number of screen pixels. Where the count rate is uniform, the value equals the image with pixel weighting enabled. It does not depend on the projection resolution, and screen pixels with no detector pixel behind them (gaps in geometric projections) no longer pull it down. Logical-view pixel weights had unit None, so dividing counts by them raised. Every logical view with pixel weighting enabled failed. The weights are now dimensionless. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ROI spectra and ROI total in range now carry the number of detector pixels in each ROI as a `detector_pixels` coord along `roi`. The separate counts per pixel output is removed. Counts per detector pixel becomes a plot option, like counts per second. That gives per-pixel spectra as well as per-pixel totals, and keeps the published values in counts. With no ROIs, the extraction takes its unit and dtype from the summed input instead of hard-coding counts, so the empty pixel count is dimensionless like the non-empty one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SimonHeybrock
force-pushed
the
roi-integrated-counts
branch
from
October 8, 2026 14:50
ec18465 to
e89875d
Compare
roi_detector_pixels took the cumulative histogram only to borrow its spectral dim for the ROI extraction, which made it recompute on every finalize. The ROI sum helper now takes any data with the two image dims first and keeps the remaining dims, so the count sums the PixelWeights directly and depends only on the projection and the ROI request. roi_spectra now checks that the spectra and the detector pixel count agree on the ROIs before attaching the count as a coord. The coord name lives in config.roi_names as DETECTOR_PIXELS_COORD, shared by the workflow, the output templates and the fake backend. A new test pins the count on a geometric projection with position-noise replicas: the full image counts the detector pixels that land on it, a small ROI can count a fraction of a pixel, and image pixels in a gap count nothing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The view sits next to total_in_range, so name it alike. The window field title follows its scalar sibling, "Total in range (update)". Field names stay roi_counts_in_range_cumulative/_current. Also: refer to "the ROI outputs" in the roi_support docstring instead of an outdated list, use the Current accumulation mode in the DetectorImage and ROISpectra docstrings, and bind the output count once in a detector service test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Adds a cross-cutting "Detector views" section with the terms behind the per-ROI detector_pixels coord, and lists "pixel" among the overloaded words. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SimonHeybrock
changed the title
SimonHeybrock
marked this pull request as ready for review
October 9, 2026 06:58
SimonHeybrock
enabled auto-merge
October 9, 2026 06:58
Member
Author
|
LGTM |
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.
Adds ROI total in range (the "integrated counts" of the TBL requirement) to every detector view that supports ROIs, and attaches the number of detector pixels in each ROI to the per-ROI outputs, so that plots can show counts per detector pixel. Together with #1369, this covers the TBL requirement for a selected ROI, including counts per pixel.
roi_spectra_*, existing)roi_total_in_range, fieldsroi_counts_in_range_*, new)Both come in cumulative and current-window variants and are published in counts.
What "in range" means
ROI total in range respects the active range filter and uses the same spectral bins as the detector image and "Total in range". The ROI spectra stay unfiltered.
Counts per detector pixel
Both outputs carry a
detector_pixelscoordinate alongroi: the number of detector pixels inside each ROI. The plot option in #1369 divides by it, so any plot of these outputs can show counts per detector pixel, alone or combined with counts per second. This makes ROIs of different size comparable, for spectra as well as for totals."Pixel" means detector pixel, not image pixel:
Bug fix
Enabling pixel weighting on a logical view raised an error, because the weights for logical views had unit
None. They are now dimensionless.Limitations
Test plan
🤖 Generated with Claude Code