Skip to content

Replace Pixel DiffCal Algorithm with Recipe #466

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 3 commits into from
Oct 14, 2024

Conversation

rboston628
Copy link
Contributor

@rboston628 rboston628 commented Oct 7, 2024

Description of work

This is part of Epic 4161

This epic requires splitting the diffcal process into the pixel part and the grouped part.

This PR is responsible for splitting off the pixel part. Consistent with a discussion on the proper use of Recipe/Algorithm, this turns the pixel diffcal process into a recipe, which is more appropriate.

Explanation of work

All of the functionality from the algorithm was moved into this recipe. The new recipe will handle its own convergence process.

The DiffractionCalibrationRecipe now calls this recipe instead of the algorithm, and does not need to call it iteratively.

All of the tests were adapted to work with this recipe. Divergences should be minimal.

Two of the tests of the DiffractioncalibrationRecipe are no longer applicable there, but are moved as tests of the PixelDiffCalRecipe.

To test

Dev testing

Check the tests. Make sure the changes are minimal and only the necessary changes to account for the new process.

The tests are changed in-place, to make it easier to review. They should be renamed and moved almost as soon as this PR is approved.

Perform the usual manual testing checks.

CIS testing

This is an enabler.

Link to EWM item

EWM#7511

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.

  • the PixelDiffractionCalibration algorithm has been replaced with a recipe
  • the tests formerly used on the algorithm work on the new recipe

@rboston628 rboston628 changed the title Ewm7388 split calibration Replace Pixel DiffCal Algorithm with Recipe Oct 7, 2024
@rboston628 rboston628 force-pushed the ewm7388-split-calibration branch 3 times, most recently from 5b41eac to 30a98d0 Compare October 9, 2024 20:42
Copy link

codecov bot commented Oct 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.51%. Comparing base (6d3cd67) to head (47d8e08).
Report is 56 commits behind head on next.

Additional details and impacted files
@@            Coverage Diff             @@
##             next     #466      +/-   ##
==========================================
+ Coverage   96.47%   96.51%   +0.04%     
==========================================
  Files          62       62              
  Lines        4510     4479      -31     
==========================================
- Hits         4351     4323      -28     
+ Misses        159      156       -3     

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

@rboston628 rboston628 marked this pull request as ready for review October 11, 2024 13:16
@rboston628 rboston628 force-pushed the ewm7388-split-calibration branch from 36a411c to be481b4 Compare October 14, 2024 14:25
@rboston628 rboston628 force-pushed the ewm7388-split-calibration branch from be481b4 to 9de3849 Compare October 14, 2024 14:28
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 DiffCal still works with the new changes.

@rboston628 rboston628 merged commit fce4acd into next Oct 14, 2024
8 checks passed
@rboston628 rboston628 deleted the ewm7388-split-calibration branch October 14, 2024 18:08
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