Repository navigation
Enable detector-view pixel weighting by default - #1368
Open
SimonHeybrock wants to merge 2 commits into
Open
SimonHeybrock wants to merge 2 commits into
SimonHeybrock wants to merge 2 commits into
Conversation
SimonHeybrock
force-pushed
the
pixel-weighting-default
branch
from
October 8, 2026 14:50
7a19de7 to
8aa4653
Compare
SimonHeybrock
force-pushed
the
roi-per-pixel-plot-option
branch
from
October 8, 2026 15:43
cae648b to
1b6c2c0
Compare
SimonHeybrock
force-pushed
the
pixel-weighting-default
branch
from
October 8, 2026 15:48
8aa4653 to
79ac4ff
Compare
Geometric projections (xy-plane, cylinder) map a varying number of detector pixels onto each image pixel, so the unweighted image shows pixel density as well as count rate. Dividing by the number of detector pixels per image pixel gives counts per detector pixel; image pixels with no detector pixel become NaN and render blank. In logical views the weight is 1 or a constant. Rewrite the setting's descriptions so the UI explains what it does, where it matters, and the empty-pixel behaviour. Fix LogicalProjector.compute_weights creating weights with unit None, which made every logical view fail with "Cannot divide with operand of unit 'None'" when weighting was enabled. The params model is the only source of the default: create_base_workflow no longer sets UsePixelWeighting, the factory always does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
State that geometric weights are averaged over the position noise, drop the claim that empty image pixels always render blank, and point to the plot option for per-pixel ROI values, which pixel weighting does not affect. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SimonHeybrock
force-pushed
the
roi-per-pixel-plot-option
branch
from
October 8, 2026 16:11
1b6c2c0 to
454f625
Compare
SimonHeybrock
force-pushed
the
pixel-weighting-default
branch
from
October 8, 2026 16:11
79ac4ff to
21118b8
Compare
This branch has not been deployed
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
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.
Stacked on #1369 (which is stacked on #1358 and #1360).
Detector views histogram detector pixels onto an image grid. With pixel weighting enabled, each image pixel is divided by the number of detector pixels that map to it, so the image shows counts per detector pixel. This PR turns weighting on by default.
Why. In geometric projections (xy-plane, cylinder) the number of detector pixels per image pixel varies across the image. Unweighted, image pixels hit by more detector pixels look brighter and gaps look dark, so the image shows pixel density as well as count rate. With weighting, image pixels that no detector pixel maps to are 0/0 = NaN and render blank instead of as zero counts. In logical views every image pixel has the same number of detector pixels: weighting is a no-op (one detector pixel per image pixel) or divides the whole image by a constant (views with
reduction_dim). Weighted images are also consistent with the "Per Detector Pixel" ROI plot option of #1369: for a uniform count rate, an ROI's counts per detector pixel equal the weighted image values inside it.Regression test. Enabling weighting on a logical view used to fail because
LogicalProjector.compute_weightscreated weights with unitNone. The fix is in #1360. This PR adds a test that runs a logical view with and withoutreduction_dim, with weighting on and off.Descriptions. The setting's title, description, and field tooltips are rewritten to say what weighting does, where it matters, and how empty image pixels render. The image output description notes that weighted counts are per detector pixel.
Single source of the default.
create_base_workflowno longer setsUsePixelWeighting = False; the factory always sets it fromparams.pixel_weighting.enabled, so the params model holds the only default.Consequences.
pixel_weighting.enabled: false. Workflows started at least once before this change keep weighting off until a user enables it or the saved config is cleared. Only workflows without a saved config pick up the new default.bank_viewand TBLngem_detector_viewimages are divided by a constant (the number of detector pixels summed into each image pixel). Other logical-view image devices are unchanged, and so are all total-count devices, which sum the histogram rather than the image.counts, so the colorbar does not show that values are per detector pixel. scipp has no unit for a detector pixel, and the image plotter cannot tell whether a job weighted its image. Follow-up: Weighted detector images are labelled counts instead of counts per detector pixel #1371.nansum), since that identity holds only for unweighted images.Test plan
reduction_dim(e.g. TBL ngem or DREAM wire view) with weighting on and check that it runs and values are counts per detector pixel.🤖 Generated with Claude Code