Enet4 / nifti-rs

Rust implementation of the NIfTI-1 format
Apache License 2.0
39 stars 11 forks source link

Writing NIfTI files #19

Open Enet4 opened 5 years ago

Enet4 commented 5 years ago

In order to write these files, we need methods for:

nilgoyette commented 5 years ago

We, at Imeka, already have the code to write a .nii or .nii.gz image and we want to contribute. It has been heavily tested (we wrote several thousands images in the last months). Of course, it's not perfect, we're not sure how to handle the scaling slope and offset, and maybe some other problems that I don't remember at the moment.

Enet4 commented 5 years ago

I would be happy to receive and review your contributions! We can do this piece by piece and discuss those concerns as they appear in the code.

nilgoyette commented 5 years ago

I forgot to tell you, my code is totally tied to ndarray. We won't be able to check any of your 3 tasks above. This being said, most of it can be re-used if this is merged. As a user of this library, I mostly use NiftiHeader and ndarray objects.

pub fn write_nifti<A, S, D>(
    path: &str,
    data: &ArrayBase<S, D>,
    reference: Option<&NiftiHeader>)
    where S: Data<Elem=A>, ...
{ ... }

Also, how to you manage temporary files in your tests? I usually use the tempfile crate but I'm pretty sure you don't want to add all those libs...

 Compiling winapi v0.3.6
 Compiling rand_core v0.3.0
 Compiling cfg-if v0.1.5
 Compiling rand_core v0.2.2
 Compiling remove_dir_all v0.5.1
 Compiling rand v0.5.5
 Compiling ansi_term v0.11.0
 Compiling pretty_assertions v0.5.1
 Compiling tempfile v3.0.4
Enet4 commented 5 years ago

I am not opposed to including a crate for temporary files in tests, especially because that would be a development dependency.

jbteves commented 3 years ago

Hi, I ran across your project while trying to make my own nifti crate (which is significantly less sophisticated...) I'd like to help out with this, are there any developer docs aside from what's on the repo?

nilgoyette commented 3 years ago

I don't think there more docs. The project is not too big so it's easy enough to jump in and modify.

We will be happy to review and merge your PRs. Is there something that you need and that nifti-rs doesn't handle?

jbteves commented 3 years ago

I'm still fiddling with it, but mostly I'm offering to help with whatever would be useful to help with writing extensions. It'll probably take me a while to make a PR since I'm very new to Rust.

twitzelbos commented 1 year ago

I just posted a PR that allows writing extensions.

twitzelbos commented 1 year ago

I guess we can now check off task 5 on the list in the original.

I have a question about the transpose in the writing of the files. In our use case we are now always pre-transposing our data, to cancel out the transpose in the writing process. What am I missing? I didn't think image data has any explicit notion of FORTRAN/C.

Enet4 commented 1 year ago

I have a question about the transpose in the writing of the files. In our use case we are now always pre-transposing our data, to cancel out the transpose in the writing process. What am I missing? I didn't think image data has any explicit notion of FORTRAN/C.

We had a discussion about this in #9 and #10 (see also comment). According to some savvy folks that were queried on this matter, plus sample data found online, NIfTI-1 volumes are required to be in column-major order on disk. This means that anyone wishing to work with them in row-major order will need to transpose the data as it arrives, then transpose it again when writing.

twitzelbos commented 1 year ago

But shouldn't the transpose then be conditional on is_standard_layout()(https://docs.rs/ndarray/latest/ndarray/struct.ArrayBase.html#method.is_standard_layout)

We are presently transposing first before calling write_nifti, so it comes out correct.

Enet4 commented 1 year ago

But shouldn't the transpose then be conditional on is_standard_layout()(https://docs.rs/ndarray/latest/ndarray/struct.ArrayBase.html#method.is_standard_layout)

The code may be under an assumption that the ndarray provided is always in C contiguous order. I cannot quite pinpoint how this is managed right now, but if it's a manual process, it should definitely become automatic. Our tests for writing various kinds of multi-dimensional arrays in tests/writer.rs seem to be doing the necessary axis inversions for the writing process to work.

With that said, what we could use here is a way to ensure that the data is in F order, which we cannot do with is_standard_layout alone. We may be able to use as_standard_layout and transpose it then.

nilgoyette commented 1 year ago

We're probably talking about that line

// Need the transpose for fortran ordering used in nifti file format.
let data = data.t();

I agree that this is strange. What is even stranger is that we tested it with C and F arrays and that it was always working properly. We're probably missing something important here. because logically it shouldn't work with F array.

In fact, I've been using this code for years at my job with both C and F 3D-4D images. We never had any problem.