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

Add new property descr., improve formulations #2256

Merged
merged 16 commits into from
Jun 30, 2022
Merged

Conversation

pwalczysko
Copy link
Member

@pwalczysko pwalczysko requested a review from sbesson June 29, 2022 16:32
Copy link
Member

@sbesson sbesson left a comment

Choose a reason for hiding this comment

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

In addition to the individual comments, we might want to review and unify the vocabulary to talk about float and double images.

As per thef formal definition of the PixelType in the OME schema:

  • float == single-precision floating point
  • double == double-precision floating point

With this in mind, I understand "floating point data types" refers to the superset of all these pixel types. Maybe floating-point data types (single or double precision) would be a better semantics to avoid the confusion float vs floating-point while clarifying there are several types?

omero/sysadmins/limitations.rst Outdated Show resolved Hide resolved
omero/sysadmins/limitations.rst Outdated Show resolved Hide resolved
generated for images with floating-point pixel data, meaning the imported
image will be scrambled if it is over the size threshold mentioned above.
generated for images with floating-point or double pixel data types, meaning the viewing of the imported
images will be compromised if it is over the size threshold mentioned above.
Copy link
Member

Choose a reason for hiding this comment

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

Main caveat with the usage of compromised is that it might be understood with other implications esp. in terms of security.

Copy link
Member

Choose a reason for hiding this comment

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

Pyramids of sub-resolution image tiles are not generated for images with floating point pixels. However, in some cases, these images can be viewed in OMERO clients at full resolution (if the images are not too large).

This behaviour is configured by omero.pixeldata.max_plane_float_override. By default this is True; OMERO overrides the requirement for floating-point images above the max_plane size to have pyramids, which allows them to be treated as regular images and possibly viewed in the clients.

However, this also allows OMERO to attempt the calculation of min/max pixel intensity for these images (normally disabled for large images because it is resource intensive to read every pixel value).

When the property omero.pixeldata.max_plane_float_override is set to False on your server, OMERO will not attempt to treat large floating-point images as if they are smaller images, so any large images without pre-generated pyramids will not be viewable. However, this will protect the server from expensive attempts to calculate min/max pixel values.

Copy link
Member

Choose a reason for hiding this comment

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

This can replace the last paragraph below

omero/sysadmins/limitations.rst Outdated Show resolved Hide resolved
@@ -79,6 +80,14 @@ This primarily affects the following file formats:
This issue can be avoided by pre-generating pyramidal OME-TIFF images as
described above.

Requests sent from clients to OMERO.server when displaying large images
with floating-point pixel data type which have no pre-generated pyramids
can cause limitation or even a complete loss of service, necessitating a server restart.
Copy link
Member

Choose a reason for hiding this comment

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

where limitations == performance issues?

@pwalczysko
Copy link
Member Author

In addition to the individual comments, we might want to review and unify the vocabulary to talk about float and double images.

As per thef formal definition of the PixelType in the OME schema:

* `float` == single-precision floating point

* `double` == double-precision floating point

With this in mind, I understand "floating point data types" refers to the superset of all these pixel types. Maybe floating-point data types (single or double precision) would be a better semantics to avoid the confusion float vs floating-point while clarifying there are several types?

Yes, definitely, thank you. I was confused about it myself

pwalczysko and others added 3 commits June 30, 2022 11:31
@@ -53,6 +53,7 @@ images if it doesn't already exist in the file. The threshold size is configurab
default. However, this process can be very resource-intensive, depending on the size of the
image as well as the image format and any data compression used, for example see
`PixelData threads and pyramid generation issues <https://forum.image.sc/t/pixeldata-threads-and-pyramid-generation-issues/49794>`_.
Further, OMERO never generates pyramids for floating-point or double pixel data types. Working with large floating-point pixel data type images without pre-generated pyramids in OMERO can cause problems (see below).
Copy link
Member

Choose a reason for hiding this comment

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

Maybe "Working with large floating-point images..." is an acceptable abbreviation? Or even "large floating-point pixel type images" is slightly less verbose

@pwalczysko
Copy link
Member Author

@will-moore @sbesson in f642440 I have included all the suggestions from both of you, as well as included a new chapter and new anchor

@pwalczysko pwalczysko requested review from sbesson and will-moore June 30, 2022 12:47
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

:model_doc:`Pyramids <omero-pyramid/>` of sub-resolution image tiles are not generated for images with floating point pixels.
However, in some cases, these images can be viewed in OMERO clients at full resolution (if the images are not too large).
Copy link
Member

Choose a reason for hiding this comment

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

Maybe remove that heading above (since we are still on the subject of Large images with floating-point pixel data types. And remove the 1st sentence of this paragraph, since it repeats what's above.
Then: "However, in some cases, large floating-point images without pyramids can be viewed..."

@pwalczysko pwalczysko requested a review from will-moore June 30, 2022 13:17
Copy link
Member

@will-moore will-moore left a comment

Choose a reason for hiding this comment

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

I think this is good-enough for this release. It's hard to know if it makes sense to a first-time reader, but at least it makes sense to me now!

Copy link
Member

@sbesson sbesson left a comment

Choose a reason for hiding this comment

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

A few suggested changes to reflow the text and end on a recommendation for large floating-point data. Agreed with @will-moore we are getting close to an acceptable first version that we can improve with time.

omero/sysadmins/limitations.rst Outdated Show resolved Hide resolved
omero/sysadmins/limitations.rst Outdated Show resolved Hide resolved
omero/sysadmins/limitations.rst Outdated Show resolved Hide resolved
omero/sysadmins/limitations.rst Outdated Show resolved Hide resolved
omero/sysadmins/limitations.rst Outdated Show resolved Hide resolved
omero/sysadmins/limitations.rst Outdated Show resolved Hide resolved
omero/sysadmins/limitations.rst Outdated Show resolved Hide resolved
omero/sysadmins/limitations.rst Outdated Show resolved Hide resolved
@pwalczysko pwalczysko requested a review from sbesson June 30, 2022 13:40
Copy link
Member

@sbesson sbesson left a comment

Choose a reason for hiding this comment

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

Thanks all, I feel comfortable linking to this section for recommendation on how to configure one server from the release announcement.

@sbesson sbesson merged commit 36cced7 into ome:develop Jun 30, 2022
@pwalczysko pwalczysko deleted the develop branch June 30, 2022 13:50
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