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

Final frame fix #82

Merged
merged 14 commits into from
Oct 15, 2024
42 changes: 17 additions & 25 deletions core/lls_core/cmds/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,20 +36,17 @@ class CliDeskewDirection(StrEnum):
"input_image": ["input_image"],
"angle": ["angle"],
"skew": ["skew"],
"pixel_sizes": ["physical_pixel_sizes"],
"rois": ["crop", "roi_list"],
"roi_indices": ["crop", "roi_subset"],
"z_start": ["crop", "z_range", 0],
"z_end": ["crop", "z_range", 1],
"physical_pixel_sizes": ["physical_pixel_sizes"],
"roi_list": ["crop", "roi_list"],
"roi_subset": ["crop", "roi_subset"],
"z_range": ["crop", "z_range"],
"decon_processing": ["deconvolution", "decon_processing"],
"psf": ["deconvolution", "psf"],
"psf_num_iter": ["deconvolution", "psf_num_iter"],
"decon_num_iter": ["deconvolution", "decon_num_iter"],
"background": ["deconvolution", "background"],
"workflow": ["workflow"],
"time_start": ["time_range", 0],
"time_end": ["time_range", 1],
"channel_start": ["channel_range", 0],
"channel_end": ["channel_range", 1],
"time_range": ["time_range"],
"channel_range": ["channel_range"],
"save_dir": ["save_dir"],
"save_name": ["save_name"],
"save_type": ["save_type"],
Expand Down Expand Up @@ -144,30 +141,25 @@ def process(
input_image: Path = Argument(None, help="Path to the image file to read, in a format readable by AICSImageIO, for example .tiff or .czi", show_default=False),
skew: CliDeskewDirection = field_from_model(DeskewParams, "skew"),# DeskewParams.make_typer_field("skew"),
angle: float = field_from_model(DeskewParams, "angle") ,
pixel_sizes: Tuple[float, float, float] = field_from_model(DeskewParams, "physical_pixel_sizes", extra_description="This takes three arguments, corresponding to the Z, Y and X pixel dimensions respectively", default=(
physical_pixel_sizes: Tuple[float, float, float] = field_from_model(DeskewParams, "physical_pixel_sizes", extra_description="This takes three arguments, corresponding to the Z, Y and X pixel dimensions respectively", default=(
DefinedPixelSizes.get_default("Z"),
DefinedPixelSizes.get_default("Y"),
DefinedPixelSizes.get_default("X")
)),

rois: List[Path] = field_from_model(CropParams, "roi_list", description="A list of paths pointing to regions of interest to crop to, in ImageJ format."), #Option([], help="A list of paths pointing to regions of interest to crop to, in ImageJ format."),
roi_indices: List[int] = field_from_model(CropParams, "roi_subset"),
# Ideally this and other range values would be defined as Tuples, but these seem to be broken: https://github.com/tiangolo/typer/discussions/667
z_start: Optional[int] = Option(0, help="The index of the first Z slice to use. All prior Z slices will be discarded.", show_default=False),
z_end: Optional[int] = Option(None, help="The index of the last Z slice to use. The selected index and all subsequent Z slices will be discarded. Defaults to the last z index of the image.", show_default=False),

roi_list: List[Path] = field_from_model(CropParams, "roi_list", description="Path pointing to regions of interest to crop to, in ImageJ ROI Manager format."), #Option([], help="A list of paths pointing to regions of interest to crop to, in ImageJ format."),
roi_subset: List[int] = field_from_model(CropParams, "roi_subset", description="List of integers describing a subset of ROIs to process. Defaults to processing all ROIs"),
z_range: Optional[Tuple[int,int]] = Option(None, help="The index of the first Z slice (inclusive) and the last z slice (exclusive) to use. All other Z slices will be discarded. Defaults to all slices.", show_default=False),

enable_deconvolution: bool = Option(False, "--deconvolution/--disable-deconvolution", rich_help_panel="Deconvolution"),
decon_processing: DeconvolutionChoice = field_from_model(DeconvolutionParams, "decon_processing", rich_help_panel="Deconvolution"),
psf: List[Path] = field_from_model(DeconvolutionParams, "psf", description="One or more paths pointing to point spread functions to use for deconvolution. Each file should in a standard image format (.czi, .tiff etc), containing a 3D image array. This option can be used multiple times to provide multiple PSF files.", rich_help_panel="Deconvolution"),
psf_num_iter: int = field_from_model(DeconvolutionParams, "psf_num_iter", rich_help_panel="Deconvolution"),
background: str = field_from_model(DeconvolutionParams, "background", rich_help_panel="Deconvolution"),
decon_num_iter: int = field_from_model(DeconvolutionParams, "decon_num_iter", description="Number of iterations of deconvolution to perform", rich_help_panel="Deconvolution"),
background: str = field_from_model(DeconvolutionParams, "background", description="Mean background value for deconvolution background subtraction, can also be 'auto' for automatic bg detection. Defaults to 0.", rich_help_panel="Deconvolution"),

time_start: Optional[int] = Option(0, help="Index of the first time slice to use (inclusive). Defaults to the first time index of the image.", rich_help_panel="Output"),
time_end: Optional[int] = Option(None, help="Index of the first time slice to use (exclusive). Defaults to the last time index of the image.", show_default=False, rich_help_panel="Output"),

channel_start: Optional[int] = Option(0, help="Index of the first channel slice to use (inclusive). Defaults to the first channel index of the image.", rich_help_panel="Output"),
channel_end: Optional[int] = Option(None, help="Index of the first channel slice to use (exclusive). Defaults to the last channel index of the image.", show_default=False, rich_help_panel="Output"),

time_range: Optional[Tuple[int,int]] = Option(None, help="Start frame (inclusive) and stop frame (exclusive) for time cropping - Defaults to full range.", rich_help_panel="Output"),
channel_range: Optional[Tuple[int,int]] = Option(None,help="Index of the first channel (inclusive) and last slice (exclusive) to use. Defaults to full range.", rich_help_panel="Output"),

save_dir: Path = field_from_model(OutputParams, "save_dir", rich_help_panel="Output"),
save_name: Optional[str] = field_from_model(OutputParams, "save_name", rich_help_panel="Output"),
save_type: SaveFileType = field_from_model(OutputParams, "save_type", rich_help_panel="Output"),
Expand Down
2 changes: 1 addition & 1 deletion core/lls_core/cropping.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def read_imagej_roi(roi_path: PathLike) -> List[Roi]:
if roi_path.suffix == ".zip":
ij_roi = read_roi_zip(roi_path)
elif roi_path.suffix == ".roi":
ij_roi = read_roi_file(roi_path)
ij_roi = read_roi_file(str(roi_path))
else:
raise Exception("ImageJ ROI file needs to be a zip/roi file")

Expand Down
2 changes: 1 addition & 1 deletion core/lls_core/models/deconvolution.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class DeconvolutionParams(FieldAccessModel):
default=[],
description="List of Point Spread Functions to use for deconvolution. Each of which should be a 3D array. Each PSF can also be provided as a `str` path, in which case they will be loaded from disk as images."
)
psf_num_iter: NonNegativeInt = Field(
decon_num_iter: NonNegativeInt = Field(
default=10,
description="Number of iterations to perform in deconvolution"
)
Expand Down
2 changes: 1 addition & 1 deletion core/lls_core/models/deskew.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ def read_image(cls, values: dict):

# If the image was convertible to AICSImage, we should use the metadata from there
if aics:
values["input_image"] = aics.xarray_dask_data
values["input_image"] = aics.xarray_dask_data
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
values["input_image"] = aics.xarray_dask_data
values["input_image"] = aics.xarray_dask_data

# Take pixel sizes from the image metadata, but only if they're defined
# and only if we don't already have them
if all(size is not None for size in aics.physical_pixel_sizes) and values.get("physical_pixel_sizes") is None:
Expand Down
22 changes: 19 additions & 3 deletions core/lls_core/models/lattice_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ def read_image(cls, values: dict):
from lls_core.types import is_pathlike
from pathlib import Path
input_image = values.get("input_image")
logger.info(f"Processing File {input_image}") # this is handy for debugging
if is_pathlike(input_image):
if values.get("save_name") is None:
values["save_name"] = Path(values["input_image"]).stem
Expand All @@ -74,6 +75,21 @@ def read_image(cls, values: dict):
# Use the Deskew version of this validator, to do the actual image loading
return super().read_image(values)

@validator("input_image", pre=True, always=True)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
@validator("input_image", pre=True, always=True)
@validator("input_image", pre=False, always=True)

def incomplete_final_frame(cls, v: DataArray) -> Any:
"""
Check final frame, if acquisition is stopped halfway through it causes failures
This validator will remove a bad final frame
"""
final_frame = v[-1]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Note that v[-1] will just take the last index along the first axis, but not the other axes. Is this what you want here?

>>> arr
<xarray.DataArray (T: 2, C: 3, Z: 4, Y: 5, X: 6)>
>>> arr[-1]
<xarray.DataArray (C: 3, Z: 4, Y: 5, X: 6)>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep that's correct. I only want to check the final frame, that's the only one that should fail in this way, I could probably drop to a single channel as well to save processing time but it's a pretty marginal improvement and this is clearer I think

Copy link
Collaborator

Choose a reason for hiding this comment

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

In that case, we could reduce the memory usage and possibly clarify what is happening by doing:

Suggested change
final_frame = v[-1]
final_frame = v.isel(T=-1, C=-1)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that is a bit more explicit, will make that change

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can click the "commit suggestion" button to use the change directly if you want.

try:
final_frame.compute()
except ValueError as e:
logger.warn("Final frame is borked. Acquisition probably stopped prematurely. Removing final frame.")
v = v[0:-1]
multimeric marked this conversation as resolved.
Show resolved Hide resolved
return v


@validator("workflow", pre=True)
def parse_workflow(cls, v: Any):
# Load the workflow from disk if it was provided as a path
Expand Down Expand Up @@ -345,7 +361,7 @@ def _process_crop(self) -> Iterable[ImageSlice]:
deconv_args: dict[Any, Any] = {}
if self.deconvolution is not None:
deconv_args = dict(
num_iter = self.deconvolution.psf_num_iter,
num_iter = self.deconvolution.decon_num_iter,
psf = self.deconvolution.psf[slice.channel].to_numpy(),
decon_processing=self.deconvolution.decon_processing
)
Expand Down Expand Up @@ -390,13 +406,13 @@ def _process_non_crop(self) -> Iterable[ImageSlice]:
dxdata=self.dx,
dzpsf=self.dz,
dxpsf=self.dx,
num_iter=self.deconvolution.psf_num_iter
num_iter=self.deconvolution.decon_num_iter
)
else:
data = skimage_decon(
vol_zyx=data,
psf=self.deconvolution.psf[slice.channel].to_numpy(),
num_iter=self.deconvolution.psf_num_iter,
num_iter=self.deconvolution.decon_num_iter,
clip=False,
filter_epsilon=0,
boundary='nearest'
Expand Down
6 changes: 3 additions & 3 deletions core/tests/test_arg_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ def test_voxel_parsing():
parser = command.make_parser(ctx)
args, _, _ = parser.parse_args(args=[
"process",
"input",
"input-image",
"--save-name", "output",
"--save-type", "tiff",
"--pixel-sizes", "1", "1", "1"
"--physical-pixel-sizes", "1", "1", "1"
])
assert args["pixel_sizes"] == ("1", "1", "1")
assert args["physical_pixel_sizes"] == ("1", "1", "1")
2 changes: 1 addition & 1 deletion core/tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def assert_h5(output_dir: Path):
[
[["--save-type", "h5"], assert_h5],
[["--save-type", "tiff"], assert_tiff],
[["--save-type", "tiff", "--time-start", "0", "--time-end", "1"], assert_tiff],
[["--save-type", "tiff", "--time-range", "0", "1"], assert_tiff],
]
)
def test_batch_deskew(flags: List[str], check_fn: Callable[[Path], None]):
Expand Down
Loading