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.

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

distortion correction preview refactoring #161

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

distortion correction preview refactoring #161

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9cf81e4
removes dev build and corrects the rescaler
dkazanc Oct 2, 2024
1712647
distortion correction preview refactoring
dkazanc Oct 7, 2024
b5e56ae
returning get_array_module call
dkazanc Oct 7, 2024
4b7def4
removing dev CI and replacing tuples with lists
dkazanc Oct 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 0 additions & 40 deletions .github/workflows/httomolibgpu_nightly_build.yml
View file
Open in desktop

This file was deleted.

40 changes: 20 additions & 20 deletions httomolibgpu/prep/alignment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
else:
map_coordinates = Mock()

from typing import Dict, List
from typing import Dict, List, Tuple, Union
Copy link
Contributor

Choose a reason for hiding this comment

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

Two things to note:

  1. Union isn't needed, regardless of if we choose a list or tuple for the shift and step params
  2. remove Tuple import if we go with a list


__all__ = [
"distortion_correction_proj_discorpy",
Expand All @@ -48,7 +48,8 @@
def distortion_correction_proj_discorpy(
data: cp.ndarray,
metadata_path: str,
preview: Dict[str, List[int]],
shift: List[Union[int, int]] = [0, 0],
step: List[Union[int, int]] = [1, 1],
Comment on lines +51 to +52
Copy link
Contributor

Choose a reason for hiding this comment

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

If we go with a list, I think the correct type would be List[int]. Currently, the type List[Union[int, int]] contains the redundant type Union[int, int] which means "an int or an int". Altogether, the type List[Union[int, int]] means "a list of values with type int or int", which I imagine is not what was intended, and the intention more likely was "a list of values of type int".

If we go with a tuple, the type should be Tuple[int, int].

order: int = 1,
mode: str = "reflect",
):
Expand All @@ -63,11 +64,11 @@ def distortion_correction_proj_discorpy(
The path to the file containing the distortion coefficients for the
data.

preview : Dict[str, List[int]]
A dict containing three key-value pairs:
- a list containing the `start` value of each dimension
- a list containing the `stop` value of each dimension
- a list containing the `step` value of each dimension
shift: List, optional
Centers of distortion in x (from the left of the image) and y directions (from the top of the image).

step: List, optional
Steps in x and y directions respectively. They need to be not larger than one.
Comment on lines +67 to +71
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to the other comment, a note about types.

If we go with a list, the type should be List[int] (no optional).

If we go with a tuple, the type should be Tuple[int, int] (no optional)


order : int, optional.
The order of the spline interpolation.
Expand All @@ -90,26 +91,25 @@ def distortion_correction_proj_discorpy(

# Use preview information to offset the x and y coords of the center of
# distortion
shift = preview["starts"]
step = preview["steps"]
x_dim = 1
y_dim = 0
step_check = max([step[i] for i in [x_dim, y_dim]]) > 1
if step_check:
det_x_step = step[0]
det_y_step = step[1]

if det_y_step > 1 or det_x_step > 1:
yousefmoazzam marked this conversation as resolved.
Show resolved Hide resolved
msg = (
"\n***********************************************\n"
"!!! ERROR !!! -> Method doesn't work with the step in"
" the preview larger than 1 \n"
"!!! ERROR !!! -> Method doesn't work with the step parameter"
" larger than 1 \n"
"***********************************************\n"
)
raise ValueError(msg)

x_offset = shift[x_dim]
y_offset = shift[y_dim]
xcenter = xcenter - x_offset
ycenter = ycenter - y_offset
det_x_shift = shift[0]
det_y_shift = shift[1]

xcenter = xcenter - det_x_shift
ycenter = ycenter - det_y_shift

height, width = data.shape[y_dim + 1], data.shape[x_dim + 1]
height, width = data.shape[1], data.shape[2]
xu_list = cp.arange(width) - xcenter
yu_list = cp.arange(height) - ycenter
xu_mat, yu_mat = cp.meshgrid(xu_list, yu_list)
Expand Down
5 changes: 3 additions & 2 deletions tests/test_prep/test_alignment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@ def test_correct_distortion(
im_host = imread(path)
im = cp.asarray(im_host)

preview = {"starts": [0, 0], "stops": [im.shape[0], im.shape[1]], "steps": [1, 1]}
corrected_data = implementation(im, distortion_coeffs_path, preview).get()
shift = (0, 0)
step = (1, 1)
corrected_data = implementation(im, distortion_coeffs_path, shift, step).get()

assert_allclose(np.mean(corrected_data), mean_value)
assert np.max(corrected_data) == max_value
Expand Down
Loading