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

MATLAB v7.3 HDF5 files use C dimension ordering #57

Open
randallpittman opened this issue May 2, 2017 · 1 comment
Open

MATLAB v7.3 HDF5 files use C dimension ordering #57

randallpittman opened this issue May 2, 2017 · 1 comment
Milestone

Comments

@randallpittman
Copy link

If I create a v7.3 file in MATLAB with a variable x = zeros([3,4,5]), and then I look at it in HDFView, it shows that it's stored in the HDF5 file with dimensions 5x4x3. So hdf5storage should not reverse the dimension order when saving to MATLAB-compatible files.

@frejanordsiek
Copy link
Owner

Matlab uses Fortran ordering for indices but stores dimensions in v7.3 MAT files with C ordered dimensions with the order of elements in memory being preserved (flat iterator).

When deciding how to implement the matlab compatibility, there was a choice to make between having arrays have the same order in memory for both Python and Matlab or to make the array look the same in each with each's individual indexing (what is in the same row in Python is in the same row in Matlab).

I chose the latter, which means a 2x5 array in Python ends up being a 2x5 array in Matlab, though the ordering in memory is different due to the required transpose to do this.

Your pull request #58 makes the other choice.

Both are legitimate choices.

I realize now that the way I wrote the package wrongly forces the one choice on the user. I think a good solution is to make the matlab_compatible not be a True/False flag but instead have three possible values, one for each of these choices and one for no matlab compatibility (would stay False); with True mapping to the current forced choice to maintain backwards compatibility. I will try to implement this soon and make a new release that adds this to the 0.1.x line and the master branch (it won't break compatibility so it can be safely added).

Thank you for bringing this oversight to my attention.

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

No branches or pull requests

2 participants