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

Handle multiple reduction runs #500

Merged
merged 3 commits into from
Nov 21, 2024
Merged

Handle multiple reduction runs #500

merged 3 commits into from
Nov 21, 2024

Conversation

rboston628
Copy link
Contributor

@rboston628 rboston628 commented Nov 18, 2024

Description of work

The Artificial Normalization screen requires the workflow to load a screen for adjusting parameters to create a false background. However, workflow's don't enable going forward or backward or optionally skipping screens, which prevents handling arbitrary cases of Artificial Normalization in run lists.

There are two "happy path" situations which may be considered, and which are enabled in this PR.

  1. user has single run requiring Artificial Normalization
  2. user has many runs, all using previously calculated normalizations from disk

The other paths have been turned off by giving the user a warning.

Explanation of work

The CalibrationService and NormalizationService were updated with the ability to load a set of calibration/normalizations for a list of run numbers. Also, return a dictionary matching a list of runs to each corresponding version.

If all runs have a matching normalization version (which is not None), then reduction works as normal over the entire list, as intended.

If any of the runs requires artificial normalization, then the workflow will throw an error dialog box requiring the user to enter only a single run.

To test

Dev testing

Setup

Download files 46802, 46803, 46805. They are in IPTS-24641 and under 1GB. These are in a new state, so use SNAPRed to create a calibration and a normalization for these runs. They do not have to be good. Just use 46803 and whichever sample and step through it until both save.

Run from WORKBENCH and not just stand-alone GUI.

Hide the normalization

Hide the normalizations you just made, most easily by changing the directory name. Run reduction with 46802, 46803, 46805. SNAPRed will not let you, and require you to clear the run list and try again with a single run.

Run reduction with just 46802. It will warn about needing artificial normalization. Continue, create an art norm, then continue to the end of the workflow.

Unhide the normalization

Close SNAPRed.

Navigate to the NormalizationIndex.json file for this state. Edit the "appliesTo" field to "46802".

Run normalization with 46802, 46803, 46805. It will complain that you must have either all or none of the runs needing artificial normalization.

Edit the "appliesTo" field to ">=46802".

Run normalization with 46802, 46803, 46805. It will run smoothly and without issue. Observe that the grouping workspaces, calibration tables, and normalizations are not being deleted, until the very end. At the end, you will have output workspaces for all three runs.

CIS testing

Follow steps similar to the above, but using the original runs 61991:61993, and valid normalizations and calibrations. They are located in /SNS/SNAP/shared/Calibration_next.

Link to EWM item

EWM#8287

See also:
EWM#8230

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.

  • When running reduction on severall runs (must be >=3 to reproduce original error) which all have valid calibrations and normalizations, SNAPRed automatically reduces all three without stopping on any screens or asking the user for further continuation
  • There are no crashes and the Mantid crash screen never comes up (run in workbench to verify this)
  • At end of reduction process, output workspaces for all three runs in the list are present.
  • The case of Artificial Normalization is handled intelligently (here, because we can only perform on one run at a time, don't let the user try with more than one run)

Copy link

codecov bot commented Nov 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.55%. Comparing base (bb31f0d) to head (1fd5b98).
Report is 2 commits behind head on next.

Additional details and impacted files
@@            Coverage Diff             @@
##             next     #500      +/-   ##
==========================================
+ Coverage   96.53%   96.55%   +0.01%     
==========================================
  Files          66       66              
  Lines        4932     4959      +27     
==========================================
+ Hits         4761     4788      +27     
  Misses        171      171              

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


🚨 Try these New Features:

@rboston628 rboston628 marked this pull request as ready for review November 21, 2024 20:16
@rboston628 rboston628 enabled auto-merge (squash) November 21, 2024 21:59
@darshdinger
Copy link
Contributor

Code looks fine. Here is the testing:

Testing


  1. Didn't download any runs, did DiffCal and NormCal for run 61991 from scratch:
    image

  2. Hide Normalization:
    image

  3. Run Reduction, but it won't let me:
    image

  4. Run Artificial Norm for only 61991:
    image

  5. Unhide norm:
    image

  6. Modify normalizationIndex.py:
    image

  7. Rerun Reduction for 61991:61993:
    image

With workspace:
image

Copy link
Contributor

@darshdinger darshdinger left a comment

Choose a reason for hiding this comment

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

Looks good.

@rboston628 rboston628 merged commit 92a9766 into next Nov 21, 2024
8 checks passed
@rboston628 rboston628 deleted the ewm8287-multirun-reduction branch November 21, 2024 23:04
rboston628 added a commit that referenced this pull request Nov 22, 2024
* get matching cal/norm versions

* update workflow warnings

* update code coverage
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