-
Notifications
You must be signed in to change notification settings - Fork 1
LogicalView for ROI views #281
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
207f62a
Implement LogicalDownsampler with transform-based design
SimonHeybrock 793ca7e
Integrate LogicalDownsampler with RollingDetectorView using duck typing
SimonHeybrock 9693e0b
Organize LogicalDownsampler tests into test classes
SimonHeybrock 3d39a2f
Replace detector_number with input_sizes in LogicalDownsampler
SimonHeybrock 819d08a
No need for end indices
SimonHeybrock 49ed893
Simplify LogicalDownsampler.input_indices() implementation
SimonHeybrock f64665b
Rename LogicalDownsampler to LogicalView and make reduction optional
SimonHeybrock f55179c
Apply suggestion from @SimonHeybrock
SimonHeybrock File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you consider adapting https://github.com/scipp/essdiffraction/blob/35310a285ba0c325def3aa3ae5795915cc10db19/src/ess/dream/diagnostics.py#L19 ? This would be able to show all voxels / pixels without user-defined projections.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is too large for streaming and imho not useful, since finding failing components in a view with >1 Mpixel is not trivial. Also I am not sure how it relates to ROIs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True. But when you combine pixels, failures can affect many display pixels and are combined with noise from working components. So seeing a failing pixel can be tricky, too.
I don't think it does unless the pixels are ordered in a meaningful way.
What kinds of ROIs is this meant for? Is this about data reduction or about diagnostics? It seems like the former because the projections you made for dream are about physical location of voxels.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not data reduction. This PR is unrelated to the DREAM PR. This is mainly for things such as ODIN, TBL, or NMX which display a 2D image detector (with our without downsampling).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ROI support is for displaying TOF spectra for the selected ROI. This is not for seeing failing pixels.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to select an actual region or a single (output) pixel? The latter could be done with a flat voxel view as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A region. And we are not doing flat voxel views in live reduction.