mobie / mobie-viewer-fiji

BSD 2-Clause "Simplified" License
28 stars 12 forks source link

making mobie projects through the UI #149

Closed K-Meech closed 3 years ago

K-Meech commented 3 years ago

@tischi @constantinpape I probably missed an issue for this somewhere - but it'd be great to be able to make new mobie projects through fiji! This is the main barrier for many people in our lab to use mobie (as they can't use Constantin's python library)

i.e. making the mobie folder structure easily / converting to bdv format / setting the starting metadata for different images...

I'm happy to help out with this, if you think it'd be useful. Thoughts?

tischi commented 3 years ago

In fact, I think this is critical if we want to publish the whole thing. It would be amazing if you could drive this!

K-Meech commented 3 years ago

Great! I have some time this week, so happy to start taking a look.

Main things would be:

Anything else I'm missing here?

constantinpape commented 3 years ago

Yes, it would be great to have this in fiji.

Anything else I'm missing here?

The initial bookmark file. Otherwise that's all we would need, I think.

K-Meech commented 3 years ago

@tischi Do you have any suggestions for the bdv conversion? I know there are plugins for converting to h5/n5 in fiji, so I'll take a look at what functions they use. Otherwise, I guess there are functions to convert random accessible intervals to h5/n5 in imglib2?

tischi commented 3 years ago

Otherwise, I guess there are functions to convert random accessible intervals to h5/n5 in imglib2?

One of those repos https://github.com/bigdataviewer should have code for ImagePlus => h5 and ImagePlus => n5. I think that's all you need, is it?

K-Meech commented 3 years ago

I guess there's an issue with the size of images. It'd be good if we could convert e.g. our big EM datasets that don't fit in RAM. But, if I use imageplus, I have to load the whole thing right?

tischi commented 3 years ago

But, if I use imageplus, I have to load the whole thing right?

Nope, you can use VirtualStack: [ File > Import >Image Sequence ] [ X ] open virtual or [ File > Import > TIFF Virtual Stack ]. And in general, everything you open with the Bio-Formats Importer can be opened virtual (lazy loading).

There was however an issue with a memory leak saving virtual stacks to h5 (@constantinpape may remember). But your efforts would be a good reason to try fixing this bug. I can help with this.

constantinpape commented 3 years ago

There was however an issue with a memory leak saving virtual stacks to h5 (@constantinpape may remember). But your efforts would be a good reason to try fixing this bug. I can help with this.

Yes, this was one of the reasons we switched to my code in the beginning, because the conversion in imagej did not work for the large platy datasets.

I am not sure how well all doing this in fji will work , for very large datasets in any case, because for this you would want to have a the cluster involved ... But already being able to create something for reasonably sized data from fiji would be super useful. And fixing memory leaks would be good in any case.

K-Meech commented 3 years ago

Ok great! Let me try with ImagePlus and virtual stacks first, and see how it works with some of our mid-sized datasets. I guess there are two ways we can go here:

Any preference here? I guess the first way is easier to handle lots of different formats e.g. image sequences...

tischi commented 3 years ago

Start with the first option. Once you have that running it should be easy to implement the second.

K-Meech commented 3 years ago

@constantinpape I'm looking at making the default tables for segmentations. Could you point me towards your code for calculating the anchors / bounding boxes? I found part of it here: https://github.com/mobie/mobie-utils-python/blob/ed0af38c41f29e49b72d61eee1cb993531cd35f1/mobie/tables/default_table.py, but I think you're using some stuff from cluster tools?

tischi commented 3 years ago

Hi @K-Meech,

@constantinpape knows this, but I think it is important to mention here that I got involved in the new image file format ome.zarr, which is very similar to n5, but the specifications will probably include storing the label image and the feature tables inside of the image container, so something like:

em.ome.zarr/raw/...
em.ome.zarr/labels/label-image
em.ome.zarr/labels/label-features

This thread is relevant: https://github.com/tischi/i2k-2020-s3-zarr-workshop/issues/16

I currently think, if possible, future versions of MoBIE should be based on ome.zarr for several reasons (we should discuss this at some point).

So, maybe have that in mind when you write your code, maybe have

interface RawImageLabelImageAndTableSaver class N5RawImageLabelImageAndGithubTableSaver implements RawImageLabelImageAndTableSaver class OMEZARRRawImageLabelImageAndTableSaver implements RawImageLabelImageAndTableSaver

😉

This probably does not actually make sense like this, but maybe you get the idea.

cc @joshmoore

K-Meech commented 3 years ago

Ok cool - I'll keep it in mind!

constantinpape commented 3 years ago

Could you point me towards your code for calculating the anchors / bounding boxes? I found part of it here: https://github.com/mobie/mobie-utils-python/blob/ed0af38c41f29e49b72d61eee1cb993531cd35f1/mobie/tables/default_table.py, but I think you're using some stuff from cluster tools?

Yes, it uses this workflow from cluster tools in order to compute the table for segmentations of arbitrary size.

But you don't need to look at the code, the statistics calculated are fairly simple:

K-Meech commented 3 years ago

Ok - thanks! @tischi - any pointers for good java implementations of centre of mass / bounding box for large label images? I imagine there are some imagej ops for this...

tischi commented 3 years ago

any pointers for good java implementations of centre of mass / bounding box for large label images?

For each label? Have a look at the function Segments3DView.setSegmentBoundingBox(...) in imagej-utils. This is the best I could find for this purpose.

K-Meech commented 3 years ago

I had a look - the issue is that, for this function, you already know which label ids are present and their centre points (from the table). Then you flood fill from this known centre to find the bounding box (I think?) I'm trying to generate the table from scratch though, so I would need to:

joshmoore commented 3 years ago

https://github.com/mobie/mobie-viewer-fiji/issues/149#issuecomment-733630815 One of those repos https://github.com/bigdataviewer should have code for...

There is also a BDVReader so technically bioformats2raw should also work. ;)

tischi commented 3 years ago

There is also a BDVReader so technically bioformats2raw should also work. ;)

@K-Meech using this would be even (much) better because then we could readily switch to ome.zarr @joshmoore Thanks a lot! Could you point Kimberly to the Java API, because I think she would not want to make system calls if it can be avoided. On the other hand for triggering conversion on the computer cluster bioformats2raw may be better but I think we also want a Fiji only solution for smaller projects.

joshmoore commented 3 years ago

👉 Converter.java is the class which drives bioformats2raw.

An example of the equivalent of bfconvert would be: https://github.com/ome/bio-formats-examples/blob/master/src/main/java/FileConvert.java

K-Meech commented 3 years ago

Great - thank you both! I'll take a look

K-Meech commented 3 years ago

@joshmoore @tischi I had a quick look, but I'm still unsure how to use it. I can call the Converter directly to give an intermediate n5/zarr file - can this then be opened directly by bdv? Or do I then need to convert this to bdv style? Also, it looks like bioformats2raw is a gradle project (I'm very new to gradle!). Is there an easy way to include it as a dependency in a maven project? Thanks!

tischi commented 3 years ago

can this then be opened directly by bdv?

In principle, yes, using code that I last week added to MoBIE-beta:

But I think currently it only works if it is stored on S3 ;-)

Can you mail me a small demo file?

K-Meech commented 3 years ago

I'm having some trouble running it. Just noticed, it needs blosc installed separately:

libblosc (https://github.com/Blosc/c-blosc) version 1.9.0 or later must be installed separately. The native libraries are not packaged with any relevant jars. 

So I'm not sure this will work for a java only solution? I think we'd need everyone to also install c-blosc?

joshmoore commented 3 years ago

Hi @K-Meech, you can turn off the compression completely via a flag. It should work then even without c-blosc installed. I imagine in the mid-term a Jar will be available that has blosc support wrapped into it.

K-Meech commented 3 years ago

Hi @tischi - I ended up refactoring some parts of 'ImagesJsonParser' to use try-with-resources blocks. https://github.com/K-Meech/mobie-viewer-fiji/blob/project_from_ui/src/main/java/de/embl/cba/mobie/image/ImagesJsonParser.java#L53 It seemed like they were leaving streams open otherwise, and interfering with creating / deleting files (at least on windows). Do you mind if I refactor the rest of the Json parsers in mobie (e.g. the bookmarks one) in the same way?

tischi commented 3 years ago

@K-Meech Fantastic initiative! Go for it! I did not even know this exists in Java, I just knew it from python with ....

K-Meech commented 3 years ago

Alright - working prototype here!: https://github.com/mobie/mobie-viewer-fiji/pull/157 Main changes summarised at the top of that pull request.

Things that need to be added in future:

joshmoore commented 3 years ago

https://github.com/mobie/mobie-viewer-fiji/issues/149#issuecomment-738078262 Is there an easy way to include it as a dependency in a maven project?

Not sure if it's still of interest but I asked at https://github.com/glencoesoftware/bioformats2raw/issues/73#issuecomment-742670631 and it looks like adding this repository may do what you want:

https://repo.glencoesoftware.com/repository/bioformats2raw2ometiff/

K-Meech commented 3 years ago

Thanks @joshmoore - this should be useful when I get around to adding ome.zarr support!

K-Meech commented 3 years ago

@tischi I've noticed that using the new commands I added changes the appearance of all swing elements shown after. e.g. before: Screenshot (195)

after: Screenshot (196)

Have you seen this before? I'm tempted to say it's something to do with using the 'File' scijava plugin parameter...

tischi commented 3 years ago

I think I have seen this before but I do not know the answer. Personally I don't mind, I can live with both.

K-Meech commented 3 years ago

I found the issue - it is something with the scijava parameters. I can fix it by resetting the default 'look and feel' for swing

tischi commented 3 years ago

I stumbled about this recent comment of Curtis in https://gitter.im/imagej/imagej

The default UI of Fiji is the LegacyUI from imagej-legacy; it has higher priority than the SwingSDIUI from imagej-ui-swing. If you exclude imagej-legacy than imagej-ui-swing's takes precedence. If you have a barebones environment without those, then the HeadlessUI of scijava-common is picked up.

Maybe related?

K-Meech commented 3 years ago

@tischi I'm trying to trigger adding images by clicking a Jbutton. Issue is - it seems that putting an IJ.run()... inside an actionListener results in it hanging forever / never finishing.

Is there a better way to IJ.run()... that avoids this? I'm calling this plugin: https://github.com/bigdataviewer/bigdataviewer_fiji/blob/master/src/main/java/bdv/ij/ExportImagePlusAsN5PlugIn.java to convert to n5. I would call it directly, but they take parameters from a general dialog, so I'm not sure how to bypass this...

tischi commented 3 years ago

In general, whatever you trigger from a UI element, I would recommend wrapping it into

ActionListener...
  SwingUtilities.invokeLater(){
     IJ.run("...");
 }

or

new Thread() ...

Or sometimes both... :-)

Does that help?

tischi commented 3 years ago

You could make a PR against this:

Proposing a version that you can call with an ImagePlus and parameters as an input.

For example like this

@Override
public void run()
{
    if ( ij.Prefs.setIJMenuBar )
        System.setProperty( "apple.laf.useScreenMenuBar", "true" );

    // get the current image
    final ImagePlus imp = WindowManager.getCurrentImage();

        // get parameters
        final Parameters params = getParameters( imp );

        export( imp, params );
}

public static void export( ImagePlus imp, Parameters params )
{
    // ... the rest of the code, without UI
}

Since I don't know if @tpietzsch would have time in the next days to look at such a PR I would recommend you to, in parallel, also copy and paste the whole class into MoBIE and make the modifications there, just to have something working.

K-Meech commented 3 years ago

Huzzah - new Thread() was the key! Thanks! I'll carry on with IJ.run() for the moment then. But you're right, in the long run having a version that could take an imp/parameters would be better.

K-Meech commented 3 years ago

Some things to fix from Yannick / Rach's test:

tischi commented 3 years ago

spaces in image names result in the table folders not being made correctly

Maybe you can auto-replace spaces by _? Or just throw an error...

K-Meech commented 3 years ago

Good point - I think auto-replacing by _ would work well

K-Meech commented 3 years ago

@tischi I'm getting a strange error. Adding an n5 image via the project creator produces the following error, but only when run through the version downloaded from the mobie beta update site. When I run this directly from intellij - no error! Any ideas? Is something perhaps different in the pom vs the update site?:

Exception in thread "Thread-6" java.lang.ClassCastException: de.embl.cba.mobie.n5.N5FSImageLoader cannot be cast to de.embl.cba.mobie.n5.N5S3ImageLoader
    at de.embl.cba.mobie.n5.XmlIoN5S3ImageLoader.toXml(XmlIoN5S3ImageLoader.java:44)
    at mpicbg.spim.data.generic.sequence.XmlIoAbstractSequenceDescription.createImgLoaderElement(XmlIoAbstractSequenceDescription.java:126)
    at mpicbg.spim.data.generic.sequence.XmlIoAbstractSequenceDescription.toXml(XmlIoAbstractSequenceDescription.java:78)
    at mpicbg.spim.data.generic.XmlIoAbstractSpimData.toXml(XmlIoAbstractSpimData.java:184)
    at mpicbg.spim.data.generic.XmlIoAbstractSpimData.save(XmlIoAbstractSpimData.java:101)
    at de.embl.cba.mobie.projects.ProjectsCreator.addBdvFormatImage(ProjectsCreator.java:137)
    at de.embl.cba.mobie.projects.ProjectsCreatorPanel.addBdvFormatImageDialog(ProjectsCreatorPanel.java:225)
    at de.embl.cba.mobie.projects.ProjectsCreatorPanel.addImageDialog(ProjectsCreatorPanel.java:140)
    at de.embl.cba.mobie.projects.ProjectsCreatorPanel.lambda$null$4(ProjectsCreatorPanel.java:95)
    at java.lang.Thread.run(Thread.java:748)
K-Meech commented 3 years ago

Ah - perhaps it is the @ImgLoaderIO syntax you're using? Does this mean it's like a plugin that will only be recognised when run in fiji proper vs in intellij?

tischi commented 3 years ago

Adding an n5 image via the project creator

  1. You mean you write an image, right?
  2. You are writing to the FileSystem (FS), right?

I am not sure...(don't have much time right now).

We have to find out which code is called by Tobias N5 writer plugin to create the xml.

In MoBIE there is this class, which has the toXml:

@ImgLoaderIo( format = "bdv.n5", type = N5FSImageLoader.class )
public class XmlIoN5FSImageLoader implements XmlIoBasicImgLoader< N5FSImageLoader >
{
    public static final String N5 = "n5";

    @Override
    public Element toXml( final N5FSImageLoader imgLoader, final File basePath )
    {
        final Element elem = new Element( "ImageLoader" );
        elem.setAttribute( IMGLOADER_FORMAT_ATTRIBUTE_NAME, "bdv.n5" );
        elem.setAttribute( "version", "1.0" );
        elem.addContent( XmlHelpers.pathElement( N5, imgLoader.getN5File(), basePath ) );
        return elem;
    }

    @Override
    public N5FSImageLoader fromXml( final Element elem, final File basePath, final AbstractSequenceDescription< ?, ?, ? > sequenceDescription )
    {
//      final String version = elem.getAttributeValue( "version" );
        final File path = loadPath( elem, N5, basePath );
        try
        {
            return new N5FSImageLoader( path, sequenceDescription );
        }
        catch ( IOException e )
        {
            throw new RuntimeException( e );
        }
    }
}

My suspicion is that we may in bdv-core there is another class with the same annotation:

@ImgLoaderIo( format = "bdv.n5", ...)

And this somehow clashes?

K-Meech commented 3 years ago

Here, I'm only writing the xml file (and linking to an existing bdv.n5 file somewhere on the file system). It uses: new XmlIoSpimDataMinimal().save()

Quite possibly a clash with e.g. bigdataviewer- https://github.com/bigdataviewer/bigdataviewer-core/blob/c698350bfae3d88b73cebc28b1f4a6fd9d5b7771/src/main/java/bdv/img/n5/XmlIoN5ImageLoader.java which has the same annotation

K-Meech commented 3 years ago

Actually @tischi - maybe the issue is here? https://github.com/mobie/mobie-viewer-fiji/blob/master/src/main/java/de/embl/cba/mobie/n5/XmlIoN5S3ImageLoader.java#L44 Should the type be N5S3ImageLoader.class instead?

tischi commented 3 years ago

Yes, of course! Please change it. Thanks!

K-Meech commented 3 years ago

Yannick / Rach have tested this - and all seems to be working well! Main thing still to add is support for ome.zarr

tischi commented 3 years ago

Main thing still to add is support for ome.zarr

Awesome! Let's try to have a zoom soon to discuss milestones. Could you try to find a date (including @joshmoore and @constantinpape please, if they can make it).

K-Meech commented 3 years ago

Sure - I'll send an email around