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

Port label support from omero-ms-image-region implementation #1

Merged
merged 7 commits into from
Mar 14, 2024
Merged

Port label support from omero-ms-image-region implementation #1

merged 7 commits into from
Mar 14, 2024

Conversation

sbesson
Copy link
Member

@sbesson sbesson commented Mar 13, 2024

This PR tried to reconcile the difference between the ZarrPixelBuffer & PixelsService implementation in this repository and the omero-ms-image-region micro-service.
Although similar, the main difference is the support for reading OME-Zarr label images specified via ExternalInfo at the Mask object which only exists in the micro-service at the moment.

These changes were made on top of 2f49133 which has the smallest diff compared to the current micro-service implementation. Below is the diff of the main source files as of ae26e1e against their equivalents in the v0.8.7 tag of omero-ms-image-region

% diff ../omero-ms-image-region/src/main/java/com/glencoesoftware/omero/ms/image/region/ZarrPixelBuffer.java src/main/java/com/glencoesoftware/omero/ms/core/ZarrPixelBuffer.java -w
19c19
< package com.glencoesoftware.omero.ms.image.region;
---
> package com.glencoesoftware.omero.ms.core;
89a90
>         log.info("Creating ZarrPixelBuffer");
714a716,719
>         if (this.resolutionLevel < 0) {
>             throw new IllegalArgumentException(
>                     "This Zarr file has no pixel data");
>         }
% diff ../omero-ms-image-region/src/main/java/com/glencoesoftware/omero/ms/image/region/OmeroAmazonS3ClientFactory.java src/main/java/com/glencoesoftware/omero/ms/core/OmeroAmazonS3ClientFactory.java -w
18c18
< package com.glencoesoftware.omero.ms.image.region;
---
> package com.glencoesoftware.omero.ms.core;
 % diff ../omero-ms-image-region/src/main/java/com/glencoesoftware/omero/ms/image/region/PixelsService.java src/main/java/com/glencoesoftware/omero/ms/core/PixelsService.java -w
19c19
< package com.glencoesoftware.omero.ms.image.region;
---
> package com.glencoesoftware.omero.ms.core;
31d30
< import java.util.Iterator;
34a34,35
> import org.perf4j.StopWatch;
> import org.perf4j.slf4j.Slf4JStopWatch;
37a39,41
> import com.github.benmanes.caffeine.cache.Cache;
> import com.github.benmanes.caffeine.cache.Caffeine;
> import com.google.common.base.Splitter;
40a45
> import ome.conditions.LockTimeout;
50,51d54
< import ome.model.screen.Well;
< import ome.model.screen.WellSample;
64d66
<     /* OME-NGFF identifiers */
73a76,84
>     /** OME NGFF LRU cache size */
>     private final long omeNgffPixelBufferCacheSize;
> 
>     /** Copy of private IQuery also provided to ome.io.nio.PixelsService */
>     private final IQuery iQuery;
> 
>     /** LRU cache of pixels ID vs OME NGFF pixel buffers */
>     private Cache<Long, ZarrPixelBuffer> omeNgffPixelBufferCache;
> 
75,77c86,90
<             String path, long memoizerWait, FilePathResolver resolver,
<             BackOff backOff, TileSizes sizes, IQuery iQuery,
<             int maxPlaneWidth, int maxPlaneHeight) throws IOException {
---
>             String path, boolean isReadOnlyRepo, File memoizerDirectory,
>             long memoizerWait, FilePathResolver resolver, BackOff backOff,
>             TileSizes sizes, IQuery iQuery,
>             long omeNgffPixelBufferCacheSize,
>             int maxPlaneWidth, int maxPlaneHeight) {
79,80c92,97
<             path, true, new File(new File(path), "BioFormatsCache"),
<             memoizerWait, resolver, backOff, sizes, iQuery);
---
>             path, isReadOnlyRepo, memoizerDirectory, memoizerWait, resolver,
>             backOff, sizes, iQuery
>         );
>         this.omeNgffPixelBufferCacheSize = omeNgffPixelBufferCacheSize;
>         log.info("OME NGFF pixel buffer cache size: {}",
>                 omeNgffPixelBufferCacheSize);
82a100,103
>         this.iQuery = iQuery;
>         omeNgffPixelBufferCache = Caffeine.newBuilder()
>                 .maximumSize(this.omeNgffPixelBufferCacheSize)
>                 .build();
141,159d161
<     }
< 
<     /**
<      * Retrieves the row, column, and field for a given set of pixels.
<      * @param pixels Set of pixels to return the row, column, and field for.
<      * @return The row, column, and field as specified by the pixels parameters
<      * or <code>null</code> if not in a plate.
<      */
<     protected int[] getRowColumnField(Pixels pixels)
<     {
<         Image image = pixels.getImage();
<         int wellSampleCount = image.sizeOfWellSamples();
<         if (wellSampleCount < 1) {
<             return null;
<         }
<         if (wellSampleCount != 1) {
<             throw new IllegalArgumentException(
<                     "Cannot resolve Image <--> Well mapping with "
<                     + "WellSample count = " + wellSampleCount);
161,173d162
<         WellSample ws = image.iterateWellSamples().next();
<         Well well = ws.getWell();
<         Iterator<WellSample> i = well.iterateWellSamples();
<         int field = 0;
<         while (i.hasNext()) {
<             WellSample v = i.next();
<             if (v.getId() == ws.getId()) {
<                 break;
<             }
<             field++;
<         }
<         return new int[] {well.getRow(), well.getColumn(), field};
<     }
175,178d163
<     @Override
<     protected int getSeries(Pixels pixels) {
<         return pixels.getImage().getSeries();
<     }
233a219,227
>      * Retrieve the {@link Image} for a particular set of pixels.
>      * @param pixels Pixels set to retrieve the {@link Image} for.
>      * @return See above.
>      */
>     protected Image getImage(Pixels pixels) {
>         return iQuery.get(Image.class, pixels.getImage().getId());
>     }
> 
>     /**
257c251
<      * Returns an NGFF pixel buffer for a given set of pixels.
---
>      * Creates an NGFF pixel buffer for a given set of pixels.
259d252
<      * @param write Whether or not to open the pixel buffer as read-write.
265c258,259
<     private PixelBuffer getOmeNgffPixelBuffer(Pixels pixels, boolean write) {
---
>     protected ZarrPixelBuffer createOmeNgffPixelBuffer(Pixels pixels) {
>         StopWatch t0 = new Slf4JStopWatch("createOmeNgffPixelBuffer()");
267c261,262
<             String uri = getUri(pixels.getImage());
---
>             Image image = getImage(pixels);
>             String uri = getUri(image);
268a264,267
>                 // Quick exit if we think we're OME-NGFF but there is no URI
>                 if ("OMEXML".equals(image.getFormat().getValue())) {
>                     throw new LockTimeout("Import in progress.", 15*1000, 0);
>                 }
272d270
<             log.info("OME-NGFF root is: " + uri);
273a272
>             log.info("OME-NGFF root is: " + uri);
275c274
<                 PixelBuffer v = new ZarrPixelBuffer(
---
>                 ZarrPixelBuffer v = new ZarrPixelBuffer(
287a287,288
>         } finally {
>             t0.stop();
304c305,306
<         PixelBuffer pixelBuffer = getOmeNgffPixelBuffer(pixels, write);
---
>         PixelBuffer pixelBuffer = omeNgffPixelBufferCache.get(
>                 pixels.getId(), key -> createOmeNgffPixelBuffer(pixels));

The main difference are PixelsService are:

  • the constructor signatures
  • the getRowColumnField API in image-region. Assuming this is an historical artifact as I cannot see where this API is being consumed
  • the usage of the caffeine cache in this component
  • the getImage and getSeries APIs
  • the name and the signature of private PixelBuffer getOmeNgffPixelBuffer(Pixels pixels, boolean write) vs protected ZarrPixelBuffer createOmeNgffPixelBuffer(Pixels pixels)
  • the fail-fast mechanism for handling OME-XML without externalnfo

@sbesson
Copy link
Member Author

sbesson commented Mar 13, 2024

@chris-allan https://github.com/sbesson/omero-ms-image-region/pull/new/omero-zarr-pixel-buffer_0.3.0 is the ongoing work to integrate this branch into the image region micro-service from this branch -

I ran into some name collision issues as omero-ms-core also ships its com.glencoesoftware.ms.core.PixelsService implementation with a different constructor signature. https://github.com/sbesson/omero-ms-core/tree/pixelsservice_removal proposes to remove the PixelsService implementation in omero-ms-core and use the one in this repository as the single source of truth. The image-region Spring configuration is modified to use the new PixelsService constructor although the review might still make build a case for subclassing PixelsService in the micro-services.

the extraction of all the Zarr + S3 dependencies into an upstream component also raises the question of whether these should be declared as api and inherited by micro-services depending on omero-zarr-pixel-buffer or kept as implementation so that they still need to be declared explicitly by consumers.

@chris-allan chris-allan marked this pull request as ready for review March 14, 2024 10:35
@chris-allan chris-allan merged commit de94b38 into glencoesoftware:master Mar 14, 2024
2 checks passed
@sbesson sbesson deleted the image_region_label_api branch March 14, 2024 10:42
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