nvladimus / npy2bdv

Fast writing of numpy 3d-arrays into HDF5 Fiji/BigDataViewer files.
GNU General Public License v3.0
34 stars 11 forks source link

per-dataset attributes not handled correctly #2

Closed VolkerH closed 4 years ago

VolkerH commented 4 years ago

While looking through the code to see how one could implement having one affine transform per dataset (see my latest comment regarding #1), I noticed that there are other attributes that can be had per-dataset that are not treated correctly.

During writing of the xml file this variable is accessed here: https://github.com/nvladimus/npy2bdv/blob/3e98e1b1dbec0d9488bc07a828e75a1f79710f29/npy2bdv.py#L152

Therefore nz, ny, nx used to write the ViewSetup subelement size (https://github.com/nvladimus/npy2bdv/blob/3e98e1b1dbec0d9488bc07a828e75a1f79710f29/npy2bdv.py#L185)

I guess what would be needed is that these parameters are passed in (or determined) for each invocation of append_view. Then some instance variables (lists) or dictionaries with keys derived from (time, ill, ch, tile, angle) could be used to keep track of these parameters for subsequent use when write_xml_file(...) is called.

VolkerH commented 4 years ago

I just had a play with this and created a proof of concept modification that allows me to assign different affine matrices to different tiles: https://github.com/VolkerH/npy2bdv/tree/perview

I have only checked this for supplying two tiles, not sure whether this would work correctly with more complicated setups involving multiple angles etc. Pretty sure it won't work properly with multiple timepoints in this form as the dictionary index is computed without the time point.

Other per-channel features would have to be added as well. My quick hack is probably enough to get the stitching project I'm currently working on done, but I'm not sure whether I find the time to bring this up to a standard where I would submit a pull request.

nvladimus commented 4 years ago

Hi, Volker, You raised good points, I was planning to add view-specific affines and other parameters. Will try to add it into the master branch this week. No need for pull request at this point, I should be able to add it on my side quite easily.

VolkerH commented 4 years ago

Great ! Looking forward. Also, I see you are aware of Talley Lambert's code. However, I'm not sure whether you are also aware of pybdv https://github.com/constantinpape/pybdv/tree/master/pybdv ?

nvladimus commented 4 years ago

I wasn't aware of pybdv, good to know, thanks!

nvladimus commented 4 years ago

I made these and some other attributes view-specific, now most attributes are added in the append_view() method, individual for every view. Check it out, and let me know how it runs!

VolkerH commented 4 years ago

Hi,

thanks for doing that so quickly. I tried the affine on multiple tiles and it appears to work for my use case - however I have only one timepoint as I want to stitch tiled images of a fixed sample.

I saw that you used a similar approach as I did in my quick hack, using the setup number as a dictionary index. This should work fine provided there is a single timepoint, however I don't see how it can work with multiple timepoints as the same setup number can be present at multiple timpoints. So the dictionary key would have to be a unique combination of timepoint + setup number to accomodate that use case.

nvladimus commented 4 years ago

Hi, Volker, Yes, I shamelessly took snippets of your code and adapted them :-) Will surely include you in the acknowledgement section! My logic regarding time points is that setup attributes usually don't change between time points. The acquisition typically cycles through all setups for every time point. So, I guess the present approach should work in most situations, except some exotic ones.

VolkerH commented 4 years ago

Nothing shameless about it :-) that's why I did it.

For my current projects, the current approach is exactly as you describe and will work just fine.

I can think of some use cases where it is problematic, e.g. "smart microscopy" with tracking acquisition, i.e. using some image analysis feedback loop that moves the stage to keep an object in the field of view. I know some people who have done this before so I'm not sure whether it qualifies as exotic.

nvladimus commented 4 years ago

I think BigStitcher XML format itself assumes that setup attributes (except the matrix transformations) are same for all time points. So it is a safe assumption for now. If there is need for time-dependent setup attributes like size etc, we can work it out later :-)