Compute Operation Flat Field #2125
Merged
Conversation
This file contains 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
ashmeigh
force-pushed
the
Compute_flat_field
branch
from
1fdbaef to
e8040be
Compare
samtygier-stfc
requested changes
Jun 11, 2024
Comment on lines
238
to
252
| def execute_wrapper( # type: ignore | ||
| flat_before_widget: DatasetSelectorWidgetView, flat_after_widget: DatasetSelectorWidgetView, | ||
| dark_before_widget: DatasetSelectorWidgetView, dark_after_widget: DatasetSelectorWidgetView, | ||
| selected_flat_fielding_widget: QComboBox, use_dark_widget: QCheckBox) -> partial: | ||
| def execute_wrapper(**kwargs): | ||
| flat_before_widget = kwargs['flat_before_widget'] | ||
| flat_after_widget = kwargs['flat_after_widget'] | ||
| dark_before_widget = kwargs['dark_before_widget'] | ||
| dark_after_widget = kwargs['dark_after_widget'] | ||
| selected_flat_fielding_widget = kwargs['selected_flat_fielding_widget'] | ||
| use_dark_widget = kwargs['use_dark_widget'] |
There was a problem hiding this comment.
These should stay as normal arguments. Going though kwargs makes type checking less useful.
Comment on lines
143
to
155
| def _divide(data, norm_divide): | ||
| np.true_divide(data, norm_divide, out=data) | ||
|
|
||
| def _subtract(data, dark=None): | ||
| # specify out to do in place, otherwise the data is copied | ||
| np.subtract(data, dark, out=data) | ||
|
|
||
| def _norm_divide(flat: np.ndarray, dark: np.ndarray) -> np.ndarray: | ||
| # subtract dark from flat | ||
| return np.subtract(flat, dark) |
There was a problem hiding this comment.
These need to be @staticmethods or to move outside the class.
| task_name='Background Correction') | ||
| _execute(images, flat_avg, dark_avg, progress) | ||
| params = {'flat_avg': flat_avg, 'dark_avg': dark_avg} | ||
| ps.run_compute_func(FlatFieldFilter.compute_function, len(images.data), [images.shared_array], params) |
There was a problem hiding this comment.
Would be good to keep the dimension checks.
|
The current set of changes have several things that seem unrelated to switching to a compute function and seem to be the source of the test errors: Changes to the logic that checks if the flats and darks are present, changes to execute_wrapper, changes to validate_execute_kwargs. |
ashmeigh
force-pushed
the
Compute_flat_field
branch
from
63a69ba to
8ea30a7
Compare
|
The 2 test fails seem odd, so I have set them to re run |
samtygier-stfc
requested changes
Jun 19, 2024
Comment on lines
49
to
62
| """Uses the flat (open beam) and dark images to normalise a stack of images (radiograms, projections), | ||
| and to correct for a beam profile, scintillator imperfections and/or detector inhomogeneities. This | ||
| operation produces images of transmission values. | ||
|
|
||
| In practice, several open beam and dark images are averaged in the flat-fielding process. | ||
|
|
||
| Intended to be used on: Projections | ||
|
|
||
| When: As one of the first pre-processing steps | ||
|
|
||
| Caution: Make sure the correct stacks are selected for flat and dark. | ||
|
|
||
| Caution: Check that the flat and dark images don't have any very bright pixels, |
There was a problem hiding this comment.
Could you avoid removing the spaces here. They are needed for nice formatting in the docs.
https://mantidproject.github.io/mantidimaging/user_guide/operations/index.html#flat-fielding
With spaces
Without spaces
Comment on lines
143
to
155
| @staticmethod | ||
| def _divide(data, norm_divide): | ||
| np.true_divide(data, norm_divide, out=data) | ||
|
|
||
| @staticmethod | ||
| def _subtract(data, dark=None): | ||
| # specify out to do in place, otherwise the data is copied | ||
| np.subtract(data, dark, out=data) | ||
|
|
||
| @staticmethod | ||
| def _norm_divide(flat: np.ndarray, dark: np.ndarray) -> np.ndarray: | ||
| # subtract dark from flat | ||
| return np.subtract(flat, dark) |
There was a problem hiding this comment.
I think these can be removed
samtygier-stfc
previously approved these changes
Jun 20, 2024
samtygier-stfc
force-pushed
the
Compute_flat_field
branch
from
afb62c8 to
6220d70
Compare
samtygier-stfc
approved these changes
Jun 20, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Issue
Closes #2051
Description
This pull request refactors the Flat Field operation to transition from the partial function style to the compute_function approach. The changes enhance code clarity, maintainability, and performance.
Changes
Refactored FlatFieldFilter: Replaced the partial function with a structured compute_function method to encapsulate the core logic.
Introduced _compute_flat_field: Encapsulated core computation logic for parallel execution.
Shared Memory Allocation: Utilized shared memory for necessary arrays to improve performance.
Testing
Verified the refactored FlatFieldFilter by running all existing unit tests.
Ensured that _compute_flat_field computes flat field normalization correctly in parallel.
Acceptance Criteria
All unit tests pass.
FlatFieldFilter performs as expected with various image inputs.
Documentation
Updated the relevant documentation to reflect changes in the FlatFieldFilter operation.
Noted all changes in the appropriate file in docs/release_notes.