Skip to content

Fiddle instrument #1761

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 33 commits into from
Mar 20, 2025
Merged

Fiddle instrument #1761

merged 33 commits into from
Mar 20, 2025

Conversation

@bnmajor
Copy link
Collaborator

@bnmajor bnmajor commented Nov 27, 2024 •
edited
Loading

  • New instrument option in LLNL Import: FIDDLE
  • Two modes: With Image PLate and Without Image Plate
  • With no image plate = load image per detector w/ dark subtraction for each image
  • With image plate = images + dark + IP w/ template & masking
  • Try to automatically find remaining images + dark after first image is loaded
  • Support manually loading images + dark if automatic find fails
  • Add hdf5 path as known path so users do not need to select it
  • Need correct default config file (See Fiddle instrument hexrd#736)
  • Need template boundary(ies) file (See Fiddle instrument hexrd#736)
  • Imageseries should say frame or timestep instead of omega
  • Image + dark = 4 frames per image file/dark file
    • Subtract each dark frame from each image frame
    • Create 4 frame imageseries from results
  • All 4 frames are in single hdf5 file. Need to handle multiple hdf5 paths for this new case
  • Need 4th config option: ATLAS coordinates - Saransh will provide script and example data (Will put up in separate PR)
  • Add new intensity correction option: median w/ kernel (default to 7; user can change)
  • On load for FIDDLE instrument pop up dialog w/ option to apply new median filter and set kernel size before Physics Package dialog

@pep8speaks
Copy link

pep8speaks commented Nov 27, 2024 •
edited
Loading

Hello @bnmajor! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 554:80: E501 line too long (95 > 79 characters)

Line 81:80: E501 line too long (83 > 79 characters)

Line 101:9: E122 continuation line missing indentation or outdented
Line 235:9: E122 continuation line missing indentation or outdented
Line 258:13: E122 continuation line missing indentation or outdented
Line 265:13: E122 continuation line missing indentation or outdented
Line 273:13: E122 continuation line missing indentation or outdented
Line 305:9: E122 continuation line missing indentation or outdented
Line 494:32: W605 invalid escape sequence '\d'
Line 495:31: W605 invalid escape sequence '\d'
Line 496:31: W605 invalid escape sequence '\d'
Line 532:13: E122 continuation line missing indentation or outdented
Line 588:13: E122 continuation line missing indentation or outdented
Line 665:9: E122 continuation line missing indentation or outdented
Line 736:9: E122 continuation line missing indentation or outdented
Line 743:9: E122 continuation line missing indentation or outdented
Line 782:9: E122 continuation line missing indentation or outdented
Line 786:9: E122 continuation line missing indentation or outdented
Line 836:9: E122 continuation line missing indentation or outdented
Line 840:9: E122 continuation line missing indentation or outdented
Line 848:9: E122 continuation line missing indentation or outdented

Comment last updated at 2025-01-03 17:29:26 UTC

@bnmajor bnmajor force-pushed the fiddle-instrument branch 5 times, most recently from 448e672 to 0bd8bfb Compare December 18, 2024 18:43
@bnmajor bnmajor force-pushed the fiddle-instrument branch from 9b17b7b to ac00f50 Compare January 3, 2025 17:29
Brianna Major and others added 20 commits February 26, 2025 09:22
@bnmajor
Add the new FIDDLE instrument
9cbea6b 
Signed-off-by: Brianna Major <[email protected]>
@bnmajor
Add hdf5 path to known paths list
96adf7b 
Signed-off-by: Brianna Major <[email protected]>
@bnmajor
Support manually loading images and dark files
26d252d 
Signed-off-by: Brianna Major <[email protected]>
@bnmajor
Try to automatically match images and dark files
4714948 
Signed-off-by: Brianna Major <[email protected]>
@bnmajor
Improve naming for clarity
78f17a2 
Update variable names to better reflect the two paths: one for image plates
and one for detectors.

Signed-off-by: Brianna Major <[email protected]>
@bnmajor
Make sure that the transform option is enabled on image load
96ded23 
Signed-off-by: Brianna Major <[email protected]>
@bnmajor
Add property to simplify check for template
eef07ba 
Signed-off-by: Brianna Major <[email protected]>
@bnmajor
Add some inline comments for clarity
7f2aee7 
Signed-off-by: Brianna Major <[email protected]>
@bnmajor
PEP8 fixes & use consistent formatting
340c575 
Signed-off-by: Brianna Major <[email protected]>
@bnmajor
Infer which values are detectors and which are image plates
05f08b0 
Signed-off-by: Brianna Major <[email protected]>
@bnmajor
Add support for median filter intensity correction
5b67dca 
It will automatically be applied once the LLNL import concludes. Kernel size
defaults to 7 but users will be prompted to change this value after import if
they'd like to. Median filtering can also be applied/removed from the
"Intensity Corrections" menu, where users will always be allowed to change the
kernel size.

Signed-off-by: Brianna Major <[email protected]>
@bnmajor
Only apply one template to an image at a time
d576bd9 
Signed-off-by: Brianna Major <[email protected]>
@bnmajor
Duplicate image plate image if it exists
0130af3 
The UI expects that all imageseries are the same length. To keep this in sync,
for now we simply create for copies of the image plate image and create the
imageseries from those frames.

Signed-off-by: Brianna Major <[email protected]>
@bnmajor
PEP8 fixes
7cdaf70 
Signed-off-by: Brianna Major <[email protected]>
@bnmajor
Add default config for FIDDLE instrument
e5ee684 
Signed-off-by: Brianna Major <[email protected]>
@psavery @bnmajor
Memoize median filter results for FIDDLE
9d2141b 
This takes up about ~200 MB to contain the results for all 21 FIDDLE
images. We may want to multi-thread across images as well...

Signed-off-by: Patrick Avery <[email protected]>
@bnmajor
Refactor median kernel into its own dialog
913f178 
Signed-off-by: Brianna Major <[email protected]>
@bnmajor
Make sure kernel size is always stored as an int
1316801 
Signed-off-by: Brianna Major <[email protected]>
@bnmajor
Do not apply the median filter during import
0b60914 
Signed-off-by: Brianna Major <[email protected]>
@bnmajor
Re-organize reset function for readability
bfff57e 
Signed-off-by: Brianna Major <[email protected]>
@bnmajor
Do not assume that dark files end with "003"
19b303d 
Signed-off-by: Brianna Major <[email protected]>
@bnmajor
Fix event check for translate/rotate template
af25d47 
We use NaNs in our template vertices in order to draw "holes" within an outer
boundary. When we check a click event to determine if a user has selected the
template or not we should only check if the outer boundary contains the event.
Using the full polygon causes some weird and inconsistent behaviors.

Signed-off-by: Brianna Major <[email protected]>
@bnmajor
Fix constant value
49224ef 
Signed-off-by: Brianna Major <[email protected]>
@bnmajor
Improve final dialogs
6bb2113 
Do not apply median filter until user sets/approves the kernel size. Do not
display physics package dialog until kernel size is set or rejected.

Signed-off-by: Brianna Major <[email protected]>
@bnmajor
Rescale colormap range when a new image is loaded
71be9ad 
Signed-off-by: Brianna Major <[email protected]>
@bnmajor
Remember images directory
8cd08aa 
Signed-off-by: Brianna Major <[email protected]>
@bnmajor
Make sure we are not transforming detector images
38024aa 
Signed-off-by: Brianna Major <[email protected]>
@bnmajor
Disable detecor image load during image plate load
ca61976 
Signed-off-by: Brianna Major <[email protected]>
@bnmajor bnmajor requested a review from psavery February 28, 2025 22:14
@bnmajor bnmajor marked this pull request as ready for review March 3, 2025 13:53
@bnmajor bnmajor mentioned this pull request Mar 14, 2025
Merged
psavery
psavery reviewed Mar 19, 2025
View reviewed changes
psavery
psavery reviewed Mar 19, 2025
View reviewed changes
psavery
psavery reviewed Mar 19, 2025
View reviewed changes
psavery
psavery reviewed Mar 19, 2025
View reviewed changes
psavery
psavery reviewed Mar 19, 2025
View reviewed changes
psavery
psavery reviewed Mar 19, 2025
View reviewed changes
psavery
psavery reviewed Mar 19, 2025
View reviewed changes
@psavery psavery mentioned this pull request Mar 19, 2025
Merged
psavery
psavery reviewed Mar 20, 2025
View reviewed changes
@bnmajor
Enforce odd values only
b40a179 
Signed-off-by: Brianna Major <[email protected]>
@bnmajor bnmajor force-pushed the fiddle-instrument branch from 4289fac to b40a179 Compare March 20, 2025 18:20
psavery
psavery approved these changes Mar 20, 2025
View reviewed changes
Copy link
Collaborator

@psavery psavery left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I tried it and verified that it works.

@psavery psavery merged commit 7aecf99 into HEXRD:master Mar 20, 2025
9 checks passed
@psavery psavery deleted the fiddle-instrument branch March 20, 2025 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@psavery psavery psavery approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bnmajor @pep8speaks @psavery