MRC import, axes mess and dtypes

[martinschorb]

Hi,

I realized that there are a couple of issues with the MRC importer.

I am not very familiar with the classes you use here, but I guess somewhere here:

elf/elf/io/mrc_wrapper.py

Line 10 in 20d7c7f

self._data = data_object

the data contained in the mrc.mfile class needs to be transposed to match numpy and other image conventions:

This is what I always do for 3D MRC tomograms to get a "well-behaving" volume:

        data = mfile.data

        data0 = np.swapaxes(data,0,2)
        data1 = np.fliplr(data0)
        data_final = np.swapaxes(data1,0,2)

Here's CD138-BMNC_01_Grid5_c0051 as an example:

Top left: original MoBIE import that I have done using my axes hack.
Top right: new MoBIE import using mobie.add_image passing it the MRC file.
Bottom left: Native IMOD viewer of the source volume.
Bottom right: BioFormats importer into ImageJ of the source volume.

You see that the axes are messed up and also the dynamic range of the view entry that is generated:

"CD138-BMNC_01_grid5_c0051": {
      "isExclusive": true,
      "sourceDisplays": [
        {
          "imageDisplay": {
            "color": "white",
            "contrastLimits": [
              0.0,
              255.0
            ],
            "name": "CD138-BMNC_01_grid5_c0051",
            "opacity": 1.0,
            "sources": [
              "CD138-BMNC_01_grid5_c0051"
            ]
          }
        }
      ],
      "uiSelectionGroup": "Tomograms"
    }

does not correctly map the original signed integer range. Does elf not provide the min and max to mobie-python?

When I adjust the viewer manually, we again have the problem of "transparent/black" zero value.

@tischi Do you know if this is getting solved any time soon?

In any case, I think it would help to have an option to map signed data types to their unsigned equivalents.
I think this should happen in elf, or where would you see this?

I don't think mrcfile can do this itself https://mrcfile.readthedocs.io/en/latest/usage_guide.html#data-types .

So I suggest the following implementations:

• import MRC files such that the axes match what is expected from the viewer
• add an option to change data type to unsigned when reading

The first point is especially crucial, otherwise all existing crop views have to be re-done because the coordinates no longer match the data.

[constantinpape]

Let me try to disentangle the two parts here:

fixing axes

Ok, this should def. be done. As far as I understand mrcfile it loads all the data into memory. So the operations to correct the axes can just be done in the init function:

elf/elf/io/mrc_wrapper.py

Line 10 in 20d7c7f

self._data = data_object

Feel free to make a PR for this, but note that you'll probably need to update the test accordingly too: https://github.com/constantinpape/elf/blob/master/test/io_tests/test_mrc_wrapper.py

value ranges

does not correctly map the original signed integer range. Does elf not provide the min and max to mobie-python?

No it doesn't and that's also not what it's there for. It should just load the data correctly through an API that's compatible with h5py. If you want something other than the default contrast limits (for the current dtype) you will need to pass them manually to add_image via the view argument. That's the same for all file types and I will also leave it this way, since the correct contrast limits should be set by the users; there is no fool-proof way to figure this out automatically. (Also note that the idea is to always treat data lazily, even if it's loaded into memory, so I can't just compute the min / max).

When I adjust the viewer manually, we again have the problem of "transparent/black" zero value.

@tischi Do you know if this is getting solved any time soon?

I guess that this is a mobie viewer issue; is there a corresponding issue about it?

In any case, I think it would help to have an option to map signed data types to their unsigned equivalents.
I think this should happen in elf, or where would you see this?

This doesn't make much sense in elf.io, since it should just do correct reading of the data without changing it. (Changing the axis format is a different matter since this is loaded the wrong way because mrcfile is doing this incorrectly.)
But we could add an option to remap dtypes to mobie-utils; I think that the underlying cluster_tools functionality supports this already.

[martinschorb]

If you want something other than the default contrast limits (for the current dtype) you will need to pass them manually to add_image via the view argument.

That's clear. In this case, the dtype is signed int8. The display limits for MoBIE still are set to [0,255]. So either the dtype is not reported correctly to mobie-python, or mobie-python ignores the fact that it is signed.

[martinschorb]

... mrcfile it loads all the data into memory.

it can actually also create a mmap (which is even the first thing the wrapper tries). But when it comes to flipping and swapping axes, numpy will deal with it regardless of being an array or a mmap.

[constantinpape]

That's clear. In this case, the dtype is signed int8. The display limits for MoBIE still are set to [0,255]. So either the dtype is not reported correctly to mobie-python, or mobie-python ignores the fact that it is signed.

Yes, mobie-python still ignores signed dtypes, since we only recently allowed for this and I haven't updated it yet to account for this. Can you make an issue about this there?

[martinschorb]

Feel free to make a PR for this, but note that you'll probably need to update the test accordingly too: https://github.com/constantinpape/elf/blob/master/test/io_tests/test_mrc_wrapper.py

for testing messy axes hacks, I will change the shape of the test array. [64,64,64] is not very useful to find out what ends up where 🙃

[constantinpape]

for testing messy axes hacks, I will change the shape of the test array. [64,64,64] is not very useful to find out what ends up where upside_down_face

Yeah, the shape should def. be changed.

[martinschorb]

here you go. Was easier than expected...
#52

[constantinpape]

I think it's all fixed by #52.