Notice: In order to edit this ticket you need to be either: a Product Owner, The owner or the reporter of the ticket, or, in case of a Task not yet assigned, a team_member"

Task #11091 (new)

Opened 6 years ago

Last modified 3 years ago

Refactor hierarchy-walking in latest Blitz FS code

Reported by: mtbcarroll Owned by: mtbcarroll
Priority: minor Milestone: Unscheduled
Component: OmeroFs Version: n.a.
Keywords: fs, graph Cc: fs@…
Resources: n.a. Referenced By: n.a.
References: n.a. Remaining Time: n.a.
Sprint: n.a.

Description

https://github.com/openmicroscopy/openmicroscopy/pull/1214 and https://github.com/openmicroscopy/openmicroscopy/pull/1235 introduce code that works well but could perform better.

Step One of this ticket's effort is that a HierarchyNavigator utility class could be factored out of most of Preprocessor except process() and then used by PojosImpl.getImagesBySplitFilesets; this class would understand the various hierarchies and walks them well by populating the query cache using HQL queries more like jamoore's Preprocessor code in retrieving both sides of the relation while using the new IN-clause batching now in PojosImpl. This is only a couple of days' work, including unit tests; I hope to fit it into the cleanup that follows the FS RC being in good enough shape for Paris, so that good query batching gets into the final 5.0 release. Once that is solved, perhaps this ticket can move to the "Cleanup" milestone.

Step Two-Alpha would be to also review jamoore's Hierarchy to identify points of commonality and perhaps to have one class use the other or to simply unify them.

Step Two-Beta is to reconsider the API. Preprocessor is happy representing hierarchy levels in an enum and working with object id properties in projections that are hopefully lightweight database queries and Java Collections that are hopefully small objects (just an enum instance and a long) that are fast to work with; the relevant code in PojosImpl also works with IDs but discussion on #11019 may change that (and resolution of that question should block this step). It's possible that this new HierarchyNavigator class should eventually be using actual class values or even (partially hydrated?) objects instead of enum-ID tuples, in the API and maybe internally too.

Change History (9)

comment:1 Changed 6 years ago by mtbcarroll

A bit of generics and subclassing in step one can make it robust to API decisions in step two.

comment:2 Changed 6 years ago by mtbcarroll

  • Milestone changed from 5.x to 5.1.0

Get step one done for 5.1.0 laying some groundwork for step two, then can push step two itself back to 5.x.

comment:3 Changed 6 years ago by mtbcarroll

Note that presently runs and well samples are glossed over: plates have wells have images. Perhaps this could be addressed.

comment:4 Changed 6 years ago by mtbcarroll

  • Milestone changed from 5.1.0 to 5.0.1

Step one should be in 5.0.1. (Soon enough to reach users before very long, but not so soon it doesn't get thoroughly tested.)

comment:5 Changed 6 years ago by mtbcarroll

  • Milestone changed from 5.0.1 to 5.x

https://github.com/openmicroscopy/openmicroscopy/pull/2005 does most of the hard work here. If it is decided to change the preprocessor API, that PR's HierarchyNavigatorWrap makes it easy given that Preprocessor already now uses a HierarchyNavigator.

It remains to check if/how jamoore's Hierarchy class can be merged with HierarchyNavigator.

comment:6 Changed 4 years ago by jamoore

  • Milestone changed from 5.x to Unscheduled

comment:7 Changed 4 years ago by mtbcarroll

  • Keywords fs graph added

comment:8 Changed 3 years ago by mtbcarroll

  • Version set to OMERO-5.2.0

It looks as if Hierarchy and HierarchyTransformations are used only by PojosImpl.findContainerHierarchies.

comment:9 Changed 3 years ago by mtbcarroll

  • Version OMERO-5.2.0 deleted
Note: See TracTickets for help on using tickets. You may also have a look at Agilo extensions to the ticket.

1.3.13-PRO © 2008-2011 Agilo Software all rights reserved (this page was served in: 0.84136 sec.)

We're Hiring!