Skip to content
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

Ewm6378 fix full masking in reduction #479

Merged
merged 9 commits into from
Oct 23, 2024

Conversation

darshdinger
Copy link
Contributor

@darshdinger darshdinger commented Oct 22, 2024

Description of work

Within testing the Reduction Workflow, it was found that errors occur during execution if a masking workspace is used where all data is masked. This causes many group based recipes within ReductionRecipe.py.

Explanation of work

The solution is to skip any algorithm requiring data for fully masked groups, provide a warning to the user, and ensure the workflow completes successfully. The skipped group will impact future reductions using that mask. Logic to handle checking the mask workspace is added to ReductionRecipe.py along with additional logic to handle skipping the group in the case the mask is masking all pixels.

To test

Dev testing

  1. Please insure that all unit tests pass.
  2. To test the functionality within SNAPRed UI, please follow the steps listed:
  • Within your local Calibration directory, delete all of your states, start SNAPRed and run a diffraction calibration and a normalization for run 58882. You can use the following inputs:

diffraction calibration
run number: 58882
convergence threshold: 0.1
bins across peak width: 10
sample: Silicon_NIST_640D_001
grouping: Column

normalization:
run number: 58882
background run number: 58810
sample: Silicon_NIST_640D_001
grouping: Column

  • Use the diffcal_masking_script.py within mantid workbench to produce a masking workspace where all pixels are masked.

NOTE: This diffcal_masking_script.py is broken! I only fixed up till L 133 to get the logic to produce a mask that can be used in testing. I have not fixed the whole cis_script as this is out of scope for this story.

  • Then before saving the mask to the disk, check the workspace data. All Y values should be 1 if you created the mask correctly. Rename it to MaskWorkspace_2 and then save it to a location easily accessible. I would recommend the Desktop.
  • Open SNAPRed once again, using the load button on the main panel, load the MaskWorkspace_2 so that it is in ADS.
  • Within the Reduction Workflow, use run number 58882, and select MaskWorkspace_2. And run.
  • Notice at the end of the workflow, a warning box pops up. Scrolling down, you will see that errors are displayed to notify user.

CIS testing

Same as dev testing but don't worry about unit tests.

Link to EWM item

EWM#6378

Verification

  • the author has read the EWM story and acceptance critera
  • the reviewer has read the EWM story and acceptance criteria
  • the reviewer certifies the acceptance criteria below reflect the criteria in EWM

Acceptance Criteria

This list is for ease of reference, and does not replace reading the EWM story as part of the review. Verify this list matches the EWM story before reviewing.

  • acceptance criterion 1
  • acceptance criterion 2

Copy link

codecov bot commented Oct 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.53%. Comparing base (6e88017) to head (b5ac8fe).
Report is 1 commits behind head on next.

Additional details and impacted files
@@            Coverage Diff             @@
##             next     #479      +/-   ##
==========================================
+ Coverage   96.52%   96.53%   +0.01%     
==========================================
  Files          63       63              
  Lines        4488     4503      +15     
==========================================
+ Hits         4332     4347      +15     
  Misses        156      156              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@darshdinger darshdinger marked this pull request as ready for review October 22, 2024 13:48
Copy link
Collaborator

@dlcaballero16 dlcaballero16 left a comment

Choose a reason for hiding this comment

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

Tested and verified Reduction workflow was able to complete even though full mask was applied. Warning messages showed as expected.

@darshdinger darshdinger merged commit 205b119 into next Oct 23, 2024
8 checks passed
@darshdinger darshdinger deleted the ewm6378-fix-full-masking-in-reduction branch October 23, 2024 13:17
rboston628 pushed a commit that referenced this pull request Oct 25, 2024
* Fix for testing on analysis.

* Fixes for test on analysis now that I've used pdb to debug.

* Test fixes.

* Update code cov

* Fix diffcal_masking_script.py

* Revert

* Revert
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants