Skip to content
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

Ensure we handle handedness correctly #10

Open
sethaxen opened this issue Apr 21, 2021 · 7 comments · May be fixed by #21
Open

Ensure we handle handedness correctly #10

sethaxen opened this issue Apr 21, 2021 · 7 comments · May be fixed by #21

Comments

@sethaxen
Copy link
Owner

From the MRC2014 spec:

The handedness of the data block is not well defined by the MRC2014 standard. Conventionally, many pieces of software have treated the data as right-handed, with the origin in the bottom left corner of a 2D image and the Z-axis pointing out of the screen.
However, this approach is not universal, and some packages treat the data block as left-handed. An example is FEI's EPU data acquisition software, which places the image origin in the top left, as documented in appendix C of the EPU User Manual.
Proposals for indicating the data handedness in the file header are under discussion, but for now, the only way to be sure of the handedness is to check the behaviour of each software package individually.

It would be bad if we got this wrong. Probably the best thing we can do is check that what we do is consistent with the mrcfile package, which is standard.

@shuuul
Copy link
Contributor

shuuul commented Jul 16, 2023

Current implementation is not consistent with mrcfile. In mrcfile, the shape of data should be (nz, ny, nx).
https://github.com/ccpem/mrcfile/blob/a0573b0ded494e4338839277570dc924c2ee512a/mrcfile/utils.py#L90

but in MRCFile.jl, the shape is (nx, ny, nz).

@sethaxen
Copy link
Owner Author

I probably will not be able to look into this again for several weeks, but I think this is because in an MRC file data is written row-major, while Julia represents data as column-major, which for multidimensional arrays reverses the order of dimensions. Perhaps we can use a PermutedDimsArray after reading the data and before writing the data for consistency.

@shuuul
Copy link
Contributor

shuuul commented Jul 17, 2023

It seems that there is no elegant way to handle it.

It has been discussed in HDF5.jl,

https://github.com/JuliaIO/HDF5.jl/blob/master/docs/src/index.md#language-interoperability-with-row--and-column-major-order-arrays
JuliaIO/HDF5.jl#785

@sethaxen
Copy link
Owner Author

It has been discussed in HDF5.jl,

Yeah, I've come across it there and with NetCDF as well. The main difference here is that in HDF5 and NetCDF the dimensions can be anything so there is no sensible default ordering, whereas in MRC they are clearly defined (from https://www.ccpem.ac.uk/mrc_format/mrc2014.php):

NX or NC | number of columns in 3D data array (fast axis)
NY or NR | number of rows in 3D data array (medium axis)
NZ or NS | number of sections in 3D data array (slow axis)

In fact, IIUC, the speed assessment of the axes in the spec seems to assume interfaces use row-major ordering. So unless there are any interfaces that provide access to MRC data with a different ordering than mrcfile's, it might make sense to always return a permuted view.

@shuuul
Copy link
Contributor

shuuul commented Jul 17, 2023

My concern is that if we permuted the data array, then read_mmap is not meaningful, but it seems we can create a view https://docs.julialang.org/en/v1/base/arrays/#Base.view.

@sethaxen
Copy link
Owner Author

I don't think it would be unmeaningful. Just like reading data from an MRC file, we would wrap the mmapped array in a PermutedDimsArray before passing it to MRCData.

But before doing anything, it's worth surveying the major packages that load MRC files and see what they do.

@shuuul
Copy link
Contributor

shuuul commented Jul 17, 2023

I see. PermutedDimsArray also creates a view for the original AbstractArray.

@shuuul shuuul linked a pull request Jul 18, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants