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

f32 images #1

Closed
nilgoyette opened this issue Oct 3, 2017 · 15 comments
Closed

f32 images #1

nilgoyette opened this issue Oct 3, 2017 · 15 comments

Comments

@nilgoyette
Copy link
Collaborator

Hi, thank you for working on this. It's the only nifti loader in rust! I tried using it to test an image denoising algo but it's not possible to load a f32 image. I know it's only a side project for you and I won't be waiting for a fix.

I tried with the simple InMemNiftiObject and by creating a NiftiHeader with the right values. Both give really sparse data. A whole lot of zeros with some random floats.

@Enet4
Copy link
Owner

Enet4 commented Oct 3, 2017

Hello! Thank you for reporting. It's true that f32 volumes should be supported, they're pretty common. Can you provide the code that you tried? I might look into this at some point.

@nilgoyette
Copy link
Collaborator Author

I created a test image with this python code

import nibabel as nib
import numpy as np

data = np.zeros((11, 11, 11), dtype=np.float32)
data[5, :, :] = np.arange(0, 1.1, 0.1)
data[:, 5, :] = np.arange(0, 1.1, 0.1)
data[:, :, 5] = np.arange(0, 1.1, 0.1)
data[5, 5, 5] = 1.0

img = nib.Nifti1Image(data, np.eye(4))
nib.save(img, '/path/to/f32.nii.gz')

And I tried to load it with

fn main() {
    let obj = InMemNiftiObject::from_file(
        "/path/to/f32.nii.gz").expect("Can't load file.");
    let header = obj.header();
    let volume = obj.volume();
    println!("{:?}", header);
    println!("{:?}", volume.dim());
    for x in 0..11 {
        for y in 0..11 {
            print!("{:?}", volume.get_f32(&[x, y, 5]).unwrap());
        }
    }
}

The header and dimension are right but the data is wrong.

@nilgoyette
Copy link
Collaborator Author

I checked InMemNiftiVolume::from_stream and I see nothing wrong with it. Must be the get_f32 then.

if self.datatype == NiftiType::Uint8 { 
    // this is working
} else { ... }

Do you already know what's missing/broken in the code for f32 images? I'm totally not a Rust expert but I would prefer fixing your lib than coding my own!

@Enet4
Copy link
Owner

Enet4 commented Oct 4, 2017

Thanks for the MCVE, I certainly wish to fix this, and I believe I have a clue of what's wrong.

@Enet4
Copy link
Owner

Enet4 commented Oct 4, 2017

All right, I just pushed some neat changes to the master branch, tests included. This latest version also allows you to convert f32 volumes to an ndarray, although it always requires a full copy right now.

@nilgoyette Would you like to try it out? There should be no breaking changes from 0.1.0.

nifti = { git = "https://github.com/Enet4/nifti-rs" }

@nilgoyette
Copy link
Collaborator Author

Whoah, thank you for working on this so fast. I'll test your branch today.

@nilgoyette
Copy link
Collaborator Author

GJ, it's working with an InMemNiftiVolume. However, it's panicking as soon as I use a ndarray:

let volume = obj.into_volume().to_ndarray::<f32>().unwrap();
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: UnsupportedDataType(Float32)', /checkout/src/libcore/result.rs:860:4

@Enet4
Copy link
Owner

Enet4 commented Oct 4, 2017

@nilgoyette Oops, that was all me. I forgot to remove an old check for the data type. The fix is in the master branch now, can you try again?

@nilgoyette
Copy link
Collaborator Author

It's compiling and there's no panic... but there's only 0.0 everywhere. You might want to add a "values test" like you did in tests/volumes.rs :)

I load an non-blank image (same as before) and the ndarray is always returning 0.0:

let volume = obj.into_volume().to_ndarray::<f32>().unwrap();
for x in 0..11 {
    for y in 0..11 {
        for z in 0..11 {
            let val = volume[[x, y, z]];
            if val != 0.0 {
                print!("{} ", val);
            }
        }
    }
}
println!("{:?}", volume.slice(s![4..7, 1..10, 1..10]));

@Enet4
Copy link
Owner

Enet4 commented Oct 4, 2017

You are right, I should indeed make a few more tests. I'll get back to this (and hopefully make a new release) in a few hours. In the mean time, can you check whether using the volume API directly works as intended?

@nilgoyette
Copy link
Collaborator Author

Yes, what I meant in my last message by "GJ, it's working with an InMemNiftiVolume" was that the values are ok. I even tested with a real brain image and I think it's right. In fact, I'm testing with

  • MITK: the values and direction fit with nifti-rs
  • nibabel: the values are ok but the directions doesn't fit with nifti-rs

Probably a LPS/RAS "problem". In fact, it's not actually a problem, I just want you to know. Does nifti-rs revert the directions or simply ignore them?

@Enet4
Copy link
Owner

Enet4 commented Oct 4, 2017

Does nifti-rs revert the directions or simply ignore them?

You are correct, orientation information stated in the header file is currently ignored (that could earn a separate issue here). Thank you for the checks, I'll try to release 0.2.0 tonight.

@Enet4
Copy link
Owner

Enet4 commented Oct 4, 2017

@nilgoyette Just to let you know that the bug has been fixed, and I will throw in a new release soon-ish.

Update: It's in.

@nilgoyette
Copy link
Collaborator Author

Sorry, I never saw your update. I'm testing with f32 images and everything seems right. I'll update this issue if I found anything.

I have a question though. The to_ndarray function only accept a pixel type and return a ndarray<T, Dyn>. If we know the image dimension, what's the proper way (or what would you do) to get a Array3<T> Do you suggest a ndarray solution, like using an ArrayView<A, 3>, or would asking a specific dimension to to_ndarray<T, D> be better?

In fact, my goal is to write something like:

let volume = ...;
if volume is in 3D
    call f(volume)
else // 4D
    for t in 0..volume.dim()[3]
        call f(volume[:, :, :, t])

fn f<A>(
    data: &Array3<A>,
    ...

@Enet4
Copy link
Owner

Enet4 commented Oct 6, 2017

Good point. I have used a dynamic shape because this one is sure to work for volumes of any dimensionality. Right now, one can use into_shape to turn it into an array with a known rank:

let a = v.to_ndarray::<f32>()?; // D = IxDyn
let shape = a.shape();
assert_eq!(shape.len(), 3);
let new_shape = [shape[0], shape[1], shape[2]];
let a = a.into_shape(new_shape)?; // D = Dim<[Ix; 3]>

On the other hand, there sure may be room for improvements in this API, and integrating a second type parameter for the dimension type might indeed work. Let's make this a separate issue. :)

@Enet4 Enet4 closed this as completed Oct 6, 2017
Enet4 pushed a commit that referenced this issue Jun 6, 2023
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

No branches or pull requests

2 participants