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

Fix SinglePlane example #124

Merged
merged 1 commit into from
May 3, 2024

Conversation

jeskowagner
Copy link
Contributor

@jeskowagner jeskowagner commented May 3, 2024

The SinglePlaneAcquisition example is currently broken in two ways:

  1. The chunk dimensions do not match the image dimensions (i.e. specify a z-channel when there is none).
  2. No distributed client is passed.
Traceback of first issue
AssertionError                            Traceback (most recent call last)
Cell In[15], line 4
      1 plate = converter.create_zarr_plate(plate_acquisition)
      3 # Run conversion.
----> 4 converter.run(
      5     plate=plate,
      6     plate_acquisition=plate_acquisition,
      7     well_sub_group="0",
      8     chunks=(1, 512, 512),
      9     max_layer=2,
     10 )

File <...>/src/faim_ipa/hcs/converter.py:152, in ConvertToNGFFPlate.run(self, plate, plate_acquisition, wells, well_sub_group, chunks, max_layer, storage_options, build_acquisition_mask)
    129 """
    130 Convert a plate acquisition to an NGFF plate.
    131 
   (...)
    149     zarr.Group of the plate.
    150 """
    151 assert 2 <= len(chunks) <= 3, "Chunks must be 2D or 3D."
--> 152 assert len(chunks) == len(
    153     plate_acquisition.get_well_acquisitions()[0].get_tiles()[0].shape
    154 ), "Chunks must have the same number of dimensions as the tile shape."
    155 well_acquisitions = plate_acquisition.get_well_acquisitions(wells)
    157 for well_acquisition in well_acquisitions:

AssertionError: Chunks must have the same number of dimensions as the tile shape.

Fixing this line to read chunks=(512, 512) lets us progress to the second issue:

Traceback of second issue
AttributeError                            Traceback (most recent call last)
File <...>/src/faim_ipa/hcs/converter.py:164, in ConvertToNGFFPlate.run(self, plate, plate_acquisition, wells, well_sub_group, chunks, max_layer, storage_options, build_acquisition_mask)
    158 well_group = self._create_well_group(
    159     plate,
    160     well_acquisition,
    161     well_sub_group,
    162 )
    163 group = well_group[well_sub_group]
--> 164 self._write_stitched_image(
    165     group,
    166     chunks,
    167     plate_acquisition,
    168     storage_options,
    169     well_acquisition,
    170     build_acquisition_mask=build_acquisition_mask,
    171 )
    172 shapes, datasets = self._build_pyramid(
    173     group,
    174     chunks,
    175     max_layer,
    176     storage_options,
    177 )
    178 self._write_metadata(
    179     group, max_layer, shapes, datasets, plate_acquisition, well_acquisition
    180 )

File <...>/src/faim_ipa/hcs/converter.py:241, in ConvertToNGFFPlate._write_stitched_image(self, group, chunks, plate_acquisition, storage_options, well_acquisition, build_acquisition_mask)
    238 rechunked_da = binned_da.rechunk(self._out_chunks(binned_da.shape, chunks))
    239 options = self._get_storage_options(storage_options, rechunked_da.shape, chunks)
    240 wait(
--> 241     self._client.persist(
    242         da.to_zarr(
    243             arr=rechunked_da,
    244             url=group.store,
    245             compute=False,
    246             component=str(Path(group.path, "0")),
    247             storage_options=options,
    248             compressor=options.get(
    249                 "compressor", zarr.storage.default_compressor
    250             ),
    251             dimension_separator=group._store._dimension_separator,
    252         ),
    253     )
    254 )

AttributeError: 'NoneType' object has no attribute 'persist'

Which seems to be triggered by converter._client being None by default. Initalising the client like in this example fixes that issue, too.

This PR includes both required changes.

@tibuch
Copy link
Contributor

tibuch commented May 3, 2024

Hi @jeskowagner,

Thanks a lot for fixing this example.

We haven't updated the examples in a long time and most likely none of them are working anymore 🙃

I am very happy to see that other people are using this as well 🥳

@tibuch tibuch merged commit af40777 into fmi-faim:main May 3, 2024
13 checks passed
@imagejan
Copy link
Member

imagejan commented May 3, 2024

Funny coincidence, I was looking at this example today as well 😄

I'd suggest to also remove the stitching_yx_chunk_size_factor=2, I'll follow up next week on this.

@jeskowagner
Copy link
Contributor Author

Yeah it seems we happened to work on this at the same time! Addressed in this new PR.

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.

3 participants