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

Volume management hackathon #1765

Closed
clbarnes opened this Issue Jul 17, 2018 · 10 comments

Comments

Projects
None yet
3 participants
@clbarnes
Copy link
Contributor

clbarnes commented Jul 17, 2018

Ideas

  • Mesh cache / model controller
  • Mesh storage (backend)
  • Mesh I/O (API)
  • Volume ontology (design)
  • Label volume support
  • Volume annotations, searches
  • Replace / improve UI in 3D viewer
  • Boolean operations on meshes
  • Visualising meshes in tracing overlay
  • Longer-term goals
    • Volume selection sets
    • Neuron meshes (alternative to skeletons as representation of neurons)

Mesh representation

  • We don't have any particular constraints based on other tools (e.g. paintera)
  • Triangle meshes are sufficient
    • Move to TIN in the backend to constrain to triangles?
    • Store a mesh as a binary blob in parallel with a bounding box for spatial queries
  • Currently no guarantees are made about meshes being closed when made with e.g. alpha shapes

EITHER

  • TIN + STL/OBJ/NGL endpoint generated on the fly
    OR
  • STL/OBJ/NGL binary storage + postgis bounding box

Formats

Binary format is required

  • obj is probably too extensive, binary format is poorly supported
  • x3d is poorly supported
  • stl is commonly supported, but contains a normal vertex and is not indexed
  • Neuroglancer format is minimalist but possibly restrictive?

Label volume support

  • Mapping integers to colours
  • Getting value of pixel under the cursor
  • Extension of stack group to persist channel etc. setup
    • Currently filters cannot be persisted

Volume annotation/search

  • Class, ClassInstance for volume/mesh
  • Extend volume manager for annotating and searching
  • Eventually abstract skeleton selection to support meshes
    • Subclasses
  • Consumer should constrain what kind of publisher it can subscribe to

3D viewer volume UI

  • Order meshes (selected on top)
  • "Featured" meshes at top
    • Use annotation
  • Eventually use mesh subscription

Mesh visualisation

  • First stab: separate webGL layer for mesh projections
  • Eventually replace pixi with threeJS for whole stack viewer

Tasks

  • Mesh storage
  • Mesh IO
    • STL import
    • STL export
    • Volume manager upload/download UI
      • Error handling for import
  • Volume ontology
    • Volume class
    • Auto-creation,-deletion of CIs and mapping to volume records
    • Migration to establish existing volumes
  • Volume annotation search
    • In volume manager
    • In 3D viewer
  • 3D viewer volume UI
    • List update on (at least locally-known) changes volume manager akin to annotation cache
    • Group active meshes at top of list (#1756)
    • Making component reusable (for future use, e.g., in tracing overlay)
      • Remove existing duplication
        • Settings widget
        • Landmark widget
        • Filtering
  • Mesh visualisation in tracing overlay

Extension

  • Label volume support

PR TODOs

  • Either fix the 0018 migration test or remove it
  • Verify winding and handedness of mesh handling
  • API changelog for all changes

tomka added a commit that referenced this issue Jul 17, 2018

Represent volumes as TIN geometry in the database
All geometries in the back-end are now stored as PostGIS TIN Geometry.
This allows for more consistency and a cleaner API. Implemented together
with @aschampion, @clbarnes, @willp24.

See #1765
Fixes #1581
@clbarnes

This comment has been minimized.

Copy link
Contributor

clbarnes commented Jul 18, 2018

meshio looks like a reasonable base if we were to start doing any significant mesh file conversion in either the backend or in catpy - we'd get a few formats for free.

I have a branch of catpy with some code to convert the x3d string into a bunch of triangles which would be helpful for either meshio or our own writers. Now that all our meshes are triangular, this is very easy.

Some simple issues with meshio which I'd want to resolve (either as PRs or on our own branch):

  • Accept pathlib.Paths in the read and write functions
  • Accept file-like objects so that they can be read or written directly from a buffer (incompatible with some formats, e.g. those based on HDF5 or netCDF)
  • Maybe some extra formats (neuroglancer, obj)
@clbarnes

This comment has been minimized.

Copy link
Contributor

clbarnes commented Jul 18, 2018

Blocked by

  • Add ability to add headers to calls in request queue
  • Expose this in CATMAID.fetch in a non-breaking way

clbarnes added a commit that referenced this issue Jul 19, 2018

Represent volumes as TIN geometry in the database
All geometries in the back-end are now stored as PostGIS TIN Geometry.
This allows for more consistency and a cleaner API. Implemented together
with @aschampion, @clbarnes, @willp24.

See #1765
Fixes #1581
@clbarnes

This comment has been minimized.

Copy link
Contributor

clbarnes commented Jul 19, 2018

Meshio is kind of lame, I'm all in on trimesh as the new hotness.

aschampion added a commit that referenced this issue Jul 20, 2018

Volumes: add volume ontology class
Add an ontology class for volumes, create class instances for all
existing volumes, and relate volume class instances to volume rows via a
`volume_class_instance` table. Also move the `model_of` relation from
the needed classes for the tracing tool to the default project needed
classes.

Written with @tomka, @clbarnes, and @willp24.

See #1765.

aschampion added a commit that referenced this issue Jul 20, 2018

@aschampion

This comment has been minimized.

Copy link
Contributor

aschampion commented Jul 20, 2018

Note that we've decided currently to not update volume class instance name's when the (spatial) volume's name is updated, since it's not clear the CI name will ever be used.

tomka added a commit that referenced this issue Jul 20, 2018

Volume manager: add annotate functionality
Add front-end and back-end functionality to add annotations to a set of
volumes. A new checkbox column has been added to the volume table and an
"Annotate" button, that allows to annotate all selected volumes.

Worked on together with @aschampion, @clbarnes and @willp24.

See #1765

pattonw pushed a commit that referenced this issue Jul 20, 2018

willp24
@aschampion

This comment has been minimized.

Copy link
Contributor

aschampion commented Jul 25, 2018

Regarding PRing this, I'm leaning towards:

  • Dropping the migration 0018 test
  • Reverting the /volumes/ endpoint change to avoid an unnecessary API breakage, even if the new response format is better
@clbarnes

This comment has been minimized.

Copy link
Contributor

clbarnes commented Jul 26, 2018

I've pushed a commit reverting the API change and its downstream usages and have left a todo.

pattonw pushed a commit to pattonw/CATMAID that referenced this issue Aug 16, 2018

Represent volumes as TIN geometry in the database
All geometries in the back-end are now stored as PostGIS TIN Geometry.
This allows for more consistency and a cleaner API. Implemented together
with @aschampion, @clbarnes, @willp24.

See catmaid#1765
Fixes catmaid#1581

pattonw pushed a commit to pattonw/CATMAID that referenced this issue Aug 16, 2018

Volumes: add volume ontology class
Add an ontology class for volumes, create class instances for all
existing volumes, and relate volume class instances to volume rows via a
`volume_class_instance` table. Also move the `model_of` relation from
the needed classes for the tracing tool to the default project needed
classes.

Written with @tomka, @clbarnes, and @willp24.

See catmaid#1765.

pattonw pushed a commit to pattonw/CATMAID that referenced this issue Aug 16, 2018

pattonw pushed a commit to pattonw/CATMAID that referenced this issue Aug 16, 2018

Volume manager: add annotate functionality
Add front-end and back-end functionality to add annotations to a set of
volumes. A new checkbox column has been added to the volume table and an
"Annotate" button, that allows to annotate all selected volumes.

Worked on together with @aschampion, @clbarnes and @willp24.

See catmaid#1765

pattonw pushed a commit to pattonw/CATMAID that referenced this issue Aug 16, 2018

tomka added a commit that referenced this issue Sep 11, 2018

Represent volumes as TIN geometry in the database
All geometries in the back-end are now stored as PostGIS TIN Geometry.
This allows for more consistency and a cleaner API. Implemented together
with @aschampion, @clbarnes, @willp24.

See #1765
Fixes #1581

tomka added a commit that referenced this issue Sep 11, 2018

Volumes: add volume ontology class
Add an ontology class for volumes, create class instances for all
existing volumes, and relate volume class instances to volume rows via a
`volume_class_instance` table. Also move the `model_of` relation from
the needed classes for the tracing tool to the default project needed
classes.

Written with @tomka, @clbarnes, and @willp24.

See #1765.

tomka added a commit that referenced this issue Sep 11, 2018

Volume manager: add annotate functionality
Add front-end and back-end functionality to add annotations to a set of
volumes. A new checkbox column has been added to the volume table and an
"Annotate" button, that allows to annotate all selected volumes.

Worked on together with @aschampion, @clbarnes and @willp24.

See #1765

tomka added a commit that referenced this issue Sep 11, 2018

@tomka

This comment has been minimized.

Copy link
Contributor

tomka commented Sep 11, 2018

I rebased the features/volume-hackathon branch on current dev and force-pushed it. Please update your local branches before you work on them. I moved the request.js and DOM related changes two the beginning and merged them into dev (039b66e, 87b1a05, aa54af4), because I have use for them in another branch. Various changes to how CATMAID.fetch() is called for the volume export have been merged into a single commit.

tomka added a commit that referenced this issue Oct 16, 2018

Represent volumes as TIN geometry in the database
All geometries in the back-end are now stored as PostGIS TIN Geometry.
This allows for more consistency and a cleaner API. Implemented together
with @aschampion, @clbarnes, @willp24.

See #1765
Fixes #1581

tomka added a commit that referenced this issue Oct 16, 2018

Volumes: add volume ontology class
Add an ontology class for volumes, create class instances for all
existing volumes, and relate volume class instances to volume rows via a
`volume_class_instance` table. Also move the `model_of` relation from
the needed classes for the tracing tool to the default project needed
classes.

Written with @tomka, @clbarnes, and @willp24.

See #1765.

tomka added a commit that referenced this issue Oct 16, 2018

Volume manager: add annotate functionality
Add front-end and back-end functionality to add annotations to a set of
volumes. A new checkbox column has been added to the volume table and an
"Annotate" button, that allows to annotate all selected volumes.

Worked on together with @aschampion, @clbarnes and @willp24.

See #1765

tomka added a commit that referenced this issue Oct 16, 2018

tomka added a commit that referenced this issue Oct 16, 2018

Represent volumes as TIN geometry in the database
All geometries in the back-end are now stored as PostGIS TIN Geometry.
This allows for more consistency and a cleaner API. Implemented together
with @aschampion, @clbarnes, @willp24.

See #1765
Fixes #1581

tomka added a commit that referenced this issue Oct 16, 2018

Volumes: add volume ontology class
Add an ontology class for volumes, create class instances for all
existing volumes, and relate volume class instances to volume rows via a
`volume_class_instance` table. Also move the `model_of` relation from
the needed classes for the tracing tool to the default project needed
classes.

Written with @tomka, @clbarnes, and @willp24.

See #1765.

tomka added a commit that referenced this issue Oct 16, 2018

Volume manager: add annotate functionality
Add front-end and back-end functionality to add annotations to a set of
volumes. A new checkbox column has been added to the volume table and an
"Annotate" button, that allows to annotate all selected volumes.

Worked on together with @aschampion, @clbarnes and @willp24.

See #1765

tomka added a commit that referenced this issue Oct 16, 2018

tomka added a commit that referenced this issue Oct 16, 2018

Volumes: add volume ontology class
Add an ontology class for volumes, create class instances for all
existing volumes, and relate volume class instances to volume rows via a
`volume_class_instance` table. Also move the `model_of` relation from
the needed classes for the tracing tool to the default project needed
classes.

Written with @tomka, @clbarnes, and @willp24.

See #1765.

tomka added a commit that referenced this issue Oct 16, 2018

Volume manager: add annotate functionality
Add front-end and back-end functionality to add annotations to a set of
volumes. A new checkbox column has been added to the volume table and an
"Annotate" button, that allows to annotate all selected volumes.

Worked on together with @aschampion, @clbarnes and @willp24.

See #1765

tomka added a commit that referenced this issue Oct 16, 2018

tomka added a commit that referenced this issue Oct 16, 2018

Volumes: add volume ontology class
Add an ontology class for volumes, create class instances for all
existing volumes, and relate volume class instances to volume rows via a
`volume_class_instance` table. Also move the `model_of` relation from
the needed classes for the tracing tool to the default project needed
classes.

Written with @tomka, @clbarnes, and @willp24.

See #1765.

tomka added a commit that referenced this issue Oct 16, 2018

Volume manager: add annotate functionality
Add front-end and back-end functionality to add annotations to a set of
volumes. A new checkbox column has been added to the volume table and an
"Annotate" button, that allows to annotate all selected volumes.

Worked on together with @aschampion, @clbarnes and @willp24.

See #1765

tomka added a commit that referenced this issue Oct 16, 2018

tomka added a commit that referenced this issue Oct 17, 2018

Volumes: add volume ontology class
Add an ontology class for volumes, create class instances for all
existing volumes, and relate volume class instances to volume rows via a
`volume_class_instance` table. Also move the `model_of` relation from
the needed classes for the tracing tool to the default project needed
classes.

Written with @tomka, @clbarnes, and @willp24.

See #1765.

tomka added a commit that referenced this issue Oct 17, 2018

Volume manager: add annotate functionality
Add front-end and back-end functionality to add annotations to a set of
volumes. A new checkbox column has been added to the volume table and an
"Annotate" button, that allows to annotate all selected volumes.

Worked on together with @aschampion, @clbarnes and @willp24.

See #1765

tomka added a commit that referenced this issue Oct 17, 2018

tomka added a commit that referenced this issue Oct 17, 2018

Volumes: add volume ontology class
Add an ontology class for volumes, create class instances for all
existing volumes, and relate volume class instances to volume rows via a
`volume_class_instance` table. Also move the `model_of` relation from
the needed classes for the tracing tool to the default project needed
classes.

Written with @tomka, @clbarnes, and @willp24.

See #1765.

tomka added a commit that referenced this issue Oct 17, 2018

Volume manager: add annotate functionality
Add front-end and back-end functionality to add annotations to a set of
volumes. A new checkbox column has been added to the volume table and an
"Annotate" button, that allows to annotate all selected volumes.

Worked on together with @aschampion, @clbarnes and @willp24.

See #1765

tomka added a commit that referenced this issue Oct 17, 2018

tomka added a commit that referenced this issue Oct 17, 2018

Volumes: add volume ontology class
Add an ontology class for volumes, create class instances for all
existing volumes, and relate volume class instances to volume rows via a
`volume_class_instance` table. Also move the `model_of` relation from
the needed classes for the tracing tool to the default project needed
classes.

Written with @tomka, @clbarnes, and @willp24.

See #1765.

tomka added a commit that referenced this issue Oct 17, 2018

Volume manager: add annotate functionality
Add front-end and back-end functionality to add annotations to a set of
volumes. A new checkbox column has been added to the volume table and an
"Annotate" button, that allows to annotate all selected volumes.

Worked on together with @aschampion, @clbarnes and @willp24.

See #1765

tomka added a commit that referenced this issue Oct 17, 2018

@tomka

This comment has been minimized.

Copy link
Contributor

tomka commented Oct 17, 2018

I rebased this a couple more times and fixed a few small things (e.g. migrations)---sorry for all the notifications due to mentions in the commit messages. I opted against removing the migration 0018 test for now, because I think it is a nice example for doing something like that (we could also keep it with an .example extension though to save the time during testing). It turned out, the problem with this test actually highlighted a problem with our migration that adds the the volume ontology elements. To do this for existing volumes in existing projects we need a system user account (because we want to add new ontology entries). Such an account doesn't exist during an initial database migration (like e.g. during testing). Instead of failing in this case, I added a try/except block that catches this situation, adds a log message and re-raises if there are volumes that would need to have a ontology counterpart created. This should only happen in very few corner cases (interrupted initial migration and added volumes before continuing migration) and during normal database initialization no message will be printed (nor exception raised) and neither will be for regular existing instances. I believe this fixes this problem well enough for now.

I also removed the latest commit, which was the reversal of the volume API change. I now agree that this change is reasonable and the new API better. This API change and all others I could find in this branch are now also explained in the API_CHANGELOG file.

Before filing a PR for this, I'd like to check the handedness of all triangles like suggested above. Other remaining issues in this ticket can be implemented in dev directly. There are many changes in here that are already very useful as is.

tomka added a commit that referenced this issue Oct 17, 2018

DB inegrity check: add test if all mesh faces are triangles
This is run now by default for the catmaid_check_db_integrity management
command.

The --tracing [true|false] and --volumes [true|false] command line
parameters now allow to explicitly test only some parts of the database.
By default all is tested.  Please enter the commit message for your
changes. Lines starting

See #1765

tomka added a commit that referenced this issue Oct 18, 2018

DB integrity check: add consistency test for volume triangle winding
If the 'trimesh' library is installed, the 'check_db_integretiy'
management command will now also check if all triangles that make up
CATMAID volumes have the same orientation (per volume).

See #1765
@tomka

This comment has been minimized.

Copy link
Contributor

tomka commented Oct 18, 2018

While I believe the TIN Geometry type in the database covers that, I added a check to the catmaid_check_db_integrety management command to check whether there are any volume components that are not triangles:

SELECT volume_id, triangle_id, path, points
FROM (
    SELECT volume_id,
        (v.gdump).path[1],
        array_agg((v.gdump).path order by (v.gdump).path[3] ASC),
        array_agg((v.gdump).geom order by (v.gdump).path[3] ASC) as points
    FROM ( 
        SELECT volume_id, gdump
        FROM ( 
            SELECT v.id AS volume_id,
                ST_DumpPoints(geometry) AS gdump
            FROM catmaid_volume v 
        ) v(volume_id, gdump)
    ) v(volume_id, gdump)
    GROUP BY v.volume_id, (v.gdump).path[1]
) triangle(volume_id, triangle_id, path, points)
WHERE array_length(points, 1) <> 4 
    OR ST_X(points[1]) <> ST_X(points[4])
    OR ST_Y(points[1]) <> ST_Y(points[4])
    OR ST_Z(points[1]) <> ST_Z(points[4]);

This should return no results and it indeed doesn't after the migrations of this branch is applied and before the migration is applied only box volumes were returned by this (which makes sense, because we stored them as polyhedral surfaces).

To test the winding, I added a second test to the catmaid_check_db_integrety management command. It tests whether the winding is consistent for each volume. If the trimesh module is not installed, this will be skipped. The check is performed by collecting the triangles for each volume and constructing a trimesh.Trimesh instance for each, which takes care of the topology so that we can ask it whether the winding is consistent. I tested that it actually does what we are expecting:

> Trimesh(vertices=np.array([[0,0,0],[1,0,0], [1,1,0], [0,1,0]]), faces=np.array([[0,1,2],[0,3,2]]), process=False).is_winding_consistent
False
> Trimesh(vertices=np.array([[0,0,0],[1,0,0], [1,1,0], [0,1,0]]), faces=np.array([[0,1,2],[0,2,3]]), process=False).is_winding_consistent
True

With the help of this management command I checked both L1 and FAFBv14 before and after the volume migrations. All volumes have a consistent winding before and after the migration is applied. I also imported different STL files and created boxes---the winding stayed consistent.

This makes me confident enough in these changes and means all the PR TODOs above are implemented, I will open a PR for the features/volume-hackathon branch. All the other changes can be implemented in dev.

@aschampion

This comment has been minimized.

Copy link
Contributor

aschampion commented Nov 10, 2018

Going to close this, as the remaining issues are now rather distinct:

  • Volume search by annotation in the 3D viewer
  • Volume visualization in the tracing overlay, which is likely another layer entirely, and in any case would best be done only if Pixi is ever ditched for Three.js in the stack viewer/tracing overlay by reuse of some of the 3D viewer (existing issue: #1434)
  • Label volume support is happening separately in #1801

@aschampion aschampion closed this Nov 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment