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

#9058 Unit support #1719

Merged
merged 19 commits into from
Nov 27, 2013
Merged

#9058 Unit support #1719

merged 19 commits into from
Nov 27, 2013

Conversation

atarkowska
Copy link
Member

This should mostly cover unit conversion in webclient, see http://trac.openmicroscopy.org.uk/ome/ticket/9058.
To test it go to webclient and check whether units on Pixel Size are correct (when you mouse over it will show real value)

Pixels Size (XYZ) (mm): 63.81 x 63.73

Main viewer should be tested parallel. Under the section Image information / Dimensions you should see exactly the same values and units.

Please test it with various images to make sure all conversions are handled properly.

Then go to exposure time on the channel and test values whether they have correct values and units

@jburel
Copy link
Member

jburel commented Nov 6, 2013

@aleksandra-tarkowska: missing support for nanometer, angstrom and picometer. I will need to find data for those ones.

@atarkowska
Copy link
Member Author

np, I wasn't sure how far we want to go.

@atarkowska
Copy link
Member Author

all units should be supported now

@will-moore
Copy link
Member

I understand the reason for these changes, but I don't think we can always know the most appropriate units just based on the value. For example (see screenshot) you would never expect to see Pixel size of a light microscope image stated in Angstroms. Only EM users will ever use Angstroms.

Also, this is a fixed image (not a timelapse) so "Timepoints: 1 second" doesn't seem right (not quite the same as 1 timepoint).

screen shot 2013-11-07 at 23 53 00

Sorry - haven't had time to track down a range of images with different scales of Z / T yet.

@jburel
Copy link
Member

jburel commented Nov 8, 2013

The unit for time should only be displayed for exposure time for example. The current implementation is not correct. This is an index

@jburel
Copy link
Member

jburel commented Nov 8, 2013

That's how it is displayed in insight
lif-insight

@atarkowska
Copy link
Member Author

I rolled back changes to time-point and adjusted exposure time. I hope is better now

@will-moore
Copy link
Member

This is still displaying Pixels Size in Angstroms instead of microns (compare with Insight screenshot above). We need to find EM images to see what their range of pixel sizes are - Then only use Angstroms in that range.

@jburel
Copy link
Member

jburel commented Nov 13, 2013

I have imported some Em data under user-6 read-only-1
insight display 0 (need to be fixed)
The value should be displayed in angstrom
emdata

@atarkowska
Copy link
Member Author

@will-moore, @jburel what are we going to do with that? Do we have any strategy for range or flag for Angstroms? If not, are we going to merge it as it is?

@will-moore
Copy link
Member

I'll import as many EM files as I can from squig - see what the pixel sizes are. What other image types do we know about that need different pixel size units? Astronomy images? Pathology images?

@jburel
Copy link
Member

jburel commented Nov 20, 2013

The image EM under user-6 will display units in Angstroms (insight)
emangstroms

@will-moore
Copy link
Member

Well, having imported a bunch of EM images from squig (from_skyking / gatan, gatan-dm2, imod, mrc, spider) it's clear that we don't do a great job of reading pixel size metadata. Either we get no size populated, or the value ranges up to hundreds of microns (not getting right units) or I can't see where the size is coming from in the Original Metadata, difficult to validate.

My gut feeling is that we should use microns -> 0.01, then nanometers 10 nm -> 1 nm, then Angstroms below that 10 A ->.

But the major blocker for nice display of pixel sizes is not formatting - it's reading the data correctly in BF.

@jburel
Copy link
Member

jburel commented Nov 20, 2013

I have also imported some fits files. no metadata in that case.
The main confusion here is the usage of nm instead of microns

@mtbc
Copy link
Member

mtbc commented Nov 20, 2013

I wonder if astronomers sometimes use fractions of degrees or radians for pixel sizes.

@jburel
Copy link
Member

jburel commented Nov 22, 2013

Need to add support for pico second cf. FLim data

@atarkowska
Copy link
Member Author

Discussed with @will-moore. As in the PR #1763 we will support micrometers up to 0.01 and then we will always do Angstrom. There is no way to guess the most appropriate unit.

@jburel
Copy link
Member

jburel commented Nov 26, 2013

@aleksandra-tarkowska, @will-moore: this is the strategy implemented in insight
jburel@61e40be

I will not go from microns to angstrom directly

@pwalczysko
Copy link
Member

I would also not leave nanometers out (nm) - this is widely used unit (unlike picometer).
I checked on the images under user-6 on Gretzky -
/Users/ola/OMERO/images/EM/qa-7642/8.dm4
the feature is confusing for 2 reasons:

  1. the Pixels size has units in (nm) = nanometers, but when you mouseover, the tooltip shows the units as micrometers (see screenshot) - no way for the user to understand what the pixel size actually is
  2. the tooltip output needs rounding up - this number of numerals is unreadable - if the exact precision is asked for, I would go for downloading it (although actually to show it in nm would get rid of those zeros at the front ?)
    screen shot 2013-11-26 at 16 12 17
    screen shot 2013-11-26 at 16 11 49

@pwalczysko
Copy link
Member

user-6 on Gretzky: spool_b._X7.061023-2.fits
The units are in both cases in micrometers, but show zero - not helpful for the user
screen shot 2013-11-26 at 16 21 43
screen shot 2013-11-26 at 16 21 33

@atarkowska
Copy link
Member Author

@pwalczysko tooltip shows you real value coming from the database as user might be interested in original value. I fixed ranges to what we all agreed as follow: use microns -> 0.01, then nanometers 10 nm -> 1 nm, then Angstroms below that 10 A ->... as the most appropriate. There is no better way of expressing 0 if pixel size is 0 in a DB ;-)

@pwalczysko
Copy link
Member

Okay, I see, rounding up is not the way to go if these are the original values. The thing is: if user is interested in original values, then probably because they want to have exact numbers for calculations. If these values have 16 digits, then they will not be helped by a tooltip - they will not be willing to write 16 digits down by hand on paper to use them for further calculations/comparisons. Not sure what the best solution is: but I would like to copy and paste these numbers somewhere.

@pwalczysko
Copy link
Member

I can see there is correspondence with Insight there about these long numbers in tooltip. Probably best to leave it now as it is.

@pwalczysko
Copy link
Member

@jburel : The following problem occurs also in Insight (as well as here in Web):
Gretzky, user-6, read-only-1, Dataset: Gatan, the first image (ID 13855):
There is a discrepancy between tooltip and the values shown in the Pixel Size box in the UI (the third value) - see screenshots.
screen shot 2013-11-27 at 10 18 44
screen shot 2013-11-27 at 10 18 36

@aleksandra-tarkowska suggested that we might merge this PR and try to deal with the problem by changing the global units concept. The problems are connected to the fact that there is a mix of units (in the example shown). @jburel : your opinion on this ?

@will-moore
Copy link
Member

@pwalczysko @jburel The fact that this gatan file has very different lengths for pixel size Z is related to my comment above "the major blocker for nice display of pixel sizes is not formatting - it's reading the data correctly in BF". That image (5nm -- BSED - slice 0010.dm4 https://gretzky.openmicroscopy.org.uk/omero/webclient/?show=image-13855) is a single Z-plane, so it doesn't make any sense to have a pixel size Z. In fact, I believe that most of the EM images I imported from squig have incorrect (or at least unvalidatable) pixel sizes. Unfortunately it is a big job to work out for sure what the values should be for all those images.

I think it is still save to assume that we can use the same units for pixels sizes X, Y and Z, although it doesn't change the fact that many OMERO DBs will have these incorrect values in them!

@jburel
Copy link
Member

jburel commented Nov 27, 2013

@aleksandra-tarkowska, @pwalczysko: we already have a rounding ticket for time but the same will apply for distance. see https://trac.openmicroscopy.org.uk/ome/ticket/11256

Having them in the tooltip was a way to show the exact value w/o having long number display. Displaying the original is not great considering that we have a limited amount of space.
@pwalczysko's requirement of copying is a different one. I will not do that in the PR

The mixing units is a problem that will need to be reviewed but no in that PR.
2 things

  • Units need to be added to the model (Strong requirement)
  • B-F reading improvement

@jburel
Copy link
Member

jburel commented Nov 27, 2013

@pwalczysko: if you are happy with the commit 35bd6cd, I will suggest that we stop there and do other changes in following PR.

@pwalczysko
Copy link
Member

Okay, happy, let us merge if all others happy as well.

joshmoore added a commit that referenced this pull request Nov 27, 2013
@joshmoore joshmoore merged commit c1359c2 into ome:develop Nov 27, 2013
@atarkowska
Copy link
Member Author

--no-rebase

@atarkowska atarkowska deleted the 9058_units branch November 27, 2013 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants