search

ome / design

OME Design proposals
http://ome.github.io/design/
1 stars 15 forks source link

ROI: Transforms #33

Closed chris-allan closed 8 years ago

chris-allan commented 8 years ago

History and state of play

ROI and Shape sub-types have supported an affine transformation matrix on their geometry since their promotion to the main OME data model from OMERO.xsd in the 2009-09 schema release. Change notes and generated schema documentation for that first pass are available:

The currently available generated documentation does not actually reflect what was present in the schema at the time so I have both linked to and included it inline here for convenience:

This is a matrix used to transform the the shape.
It is a string of 6 numbers "a, b, c, d, e, f" that
represent the 3 by 3 matrix.
--       --
| a, c, e |
| b, c, f |
| 0, 0, 1 |
--       --

The legacy of these attributes and elements is that of the RoiDisplayOptions element introduced as part of OMERO.xsd in the 2008-09 schema release. Change notes and generated schema documentation are also available for that:

As referenced in the change notes, the inspiration for RoiDisplayOptions was that of SVG. SVG's "transform" attribute, which can be chained as part of a transform list is fully documented and can be reviewed:

To my knowledge no version of RoiDisplayOptions was ever reflected in a released version of the OMERO data model.

That said, given the legacy of both the attributes and elements themselves and their SVG inspiration it shouldn't surprise us too much that this SVG style is the form by which OMERO.insight's measurement tool began populating them as part of the Beta4.1.0 release of OMERO in October, 2009. The mapping file for the OMERO data model at the time is available:

Unfortunately this is at odds with the schema and persists to this day:

OMERO.web works around these idiosyncrasies in JavaScript:

It is handling a case, rotate(...), which is no longer populated by OMERO.insight but is legal SVG style. As a rotation can be handled by the matrix(...) style it was likely superceded.

No changes to how OMERO.insight has been populating the transform have taken place since initially committed in October, 2009 (openmicroscopy/openmicroscopy@332cd539e48ee22996467ad39e2476e0ea63c8f4).

The current list of potential forms for the transform as reflected in the OMERO.insight codebase are:

When a transform is coming through the import process and passes through the series of MetadataStore implementations, it has an additional form:

This is reflected in the implementation:

The final location within the OMERO codebase where transforms are referenced is within the now deprecated OmeroPopo package:

Since the OME data model 2012-06 schema release, Shape sub-types have a much more structured markup surrounding the transform property. This is reflected in the current developer documentation which is available here:

The change notes and, before and after sets of generated schema documentation are available:

No changes to the OMERO data model representation of transforms have taken place since their introduction.

The issue

The current state of how Shape sub-type transforms are modelled in the OME or OMERO data model would be of much lesser importance if OMERO.insight were not populating the transform on every creation or update (admittedly often innocuously as none), and utilizing it exclusively for performing translations of ellipses.

Both utilizing ellipses that have been moved in OMERO.insight as a third-party or making geometric, extended or otherwise, queries against the database then must take transforms into account. This is made even more difficult by their current string syntax and worse by the fact that documentation on the form that these transforms might take is contradictory.

Suggestions for moving forward

OMERO 5.2.x

/cc @jburel, @joshmoore, @sbesson, @will-moore, @mtbc, @hflynn

/cc @emilroz, @knabar

Edits

jburel commented 8 years ago

This might imply a 5.2.4 since we won't be able to do the work on time for 5.2.3

mtbc commented 8 years ago

There's more code at https://github.com/openmicroscopy/openmicroscopy/blob/v5.2.2/components/blitz/src/ome/services/blitz/impl/OmeroMetadata.java#L483 which may need attention.

mtbc commented 8 years ago

When this issue is decomposed into Trello cards feel free to assign me the DB upgrade script, it'll probably be non-trivial.

mtbc commented 8 years ago

I am wondering if for 5.3 the OMERO representation of a transform should actually be a nullable list/vector of six numbers and, if so, if we would still need a string representation anywhere. (One could even make the numbers named properties like in the XML if that's warranted.)

mtbc commented 8 years ago

not encourage the use of the transform for translations that could be performed by updating the core attributes

I wonder if the server should automatically rewrite such transforms away when the shape is saved.

mtbc commented 8 years ago

The current list of potential forms for the transform ...

It'd be great if someone with access to diverse production databases could collect a couple of concrete examples of each form that I could use in testing PL/pgSQL code; it'd be good not to miss any. (Given a SELECT DISTINCT transform FROM Shape I'm happy to dig them out myself from the gzipped results.)

joshmoore commented 8 years ago

nightshade:

omero=# select x, count(x) from (select case
     when transform like 'translate(%' then 'translate'
     when transform like 'matrix(%' then 'matrix' 
     else transform end  as "x" from shape) q group by x;

                x                | count
---------------------------------+--------
                                 |      0
 matrix                          |  10639
 translate                       |  33277
 rotate(0 262.000000 174.000000) |      4
 none                            | 194597
(5 rows)

transforms.gz

mtbc commented 8 years ago

Excellent, thanks. :+1: Followup, sorry: the ones that take arguments have number/syntax we'd expect from the description? (E.g., translate and scale have one or two. Hmm, I wonder if one means the second is the same.) Update: gah, now noticed attachment, thank you again.

chris-allan commented 8 years ago

One limited example:

omero=> select x, count(x) from (select case
omero(>      when transform like 'translate(%' then 'translate'
omero(>      when transform like 'matrix(%' then 'matrix' 
omero(>      else transform end  as "x" from shape) q group by x;
   x    | count 
--------+-------
 matrix |     1
        |     0
 none   |   429
(3 rows)

Attached transforms from:

omero=> \copy (select transform from shape where transform is not null) to 'transform.csv' with CSV

transform.csv.gz

mtbc commented 8 years ago

In reviewing the above I find that nightshade has some interesting extras:

also examples of scientific notation, e.g., matrix(0.9999999999999984 0 0 1 9.237055564881302E-14 0), which PostgreSQL can already handle.

joshmoore commented 8 years ago

many \N -- is that a literal line feed? What's creating these?

These are produced by copy to stdout with no other options. I.e. NULL.

mtbc commented 8 years ago

Aha, thank you, they're just nulls.

mtbc commented 8 years ago

I wonder if we have to handle rotations around non-zero angles!

mtbc commented 8 years ago

if OMERO.insight were not populating the transform on every creation or update (admittedly often innocuously as none), and utilizing it exclusively for performing translations of ellipses

I'd like to see some OME-XML with transforms in it but I can't figure out how to reproduce the above issue with current Insight. @dominikl, did you fix it; maybe I need to use an older version? Do we have any example OME-XML that does have transforms; @sbesson, are you aware of any suitable samples?

dominikl commented 8 years ago

I'm not aware that there has been something recently fixed in Insight with respect to transforms.

mtbc commented 8 years ago

In my unusual blindness @sbesson kindly points me to that some of the OME-XML in the BF samples directory have them.

sbesson commented 8 years ago

See also https://github.com/ome/blog/pull/34 for a blog post describing the transform status and the projected braking changes in OMERO 5.3.

mtbc commented 8 years ago

Do we need more 5.2 work done here or maybe this issue can now be closed?

sbesson commented 8 years ago

As we decided to address this issue in OMERO 5.3.0 including SQL scripts, I do not think any 5.2 work is planned/necessary. From the original description, the only remaining question is whether the documentation effort for the breaking change has been scheduled accordingly.

mtbc commented 8 years ago

Not sure if https://trello.com/c/PEvlrmHX/36-document-breaking-api-changes-for-devs-add-new-examples suffices or more is in mind.

hflynn commented 8 years ago

I've added reference to this on the card @mtbc links above

sbesson commented 8 years ago

Thanks both. Closing this then. We can reopen if we require more discussions.