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

bug with Integer valued niftis #31

Open
vsaase opened this issue Sep 11, 2019 · 9 comments
Open

bug with Integer valued niftis #31

vsaase opened this issue Sep 11, 2019 · 9 comments

Comments

@vsaase
Copy link

vsaase commented Sep 11, 2019

When creating a Integer valued nifti:
ni = NIVolume(Int16.(round.(rand(2,2,2))))
its type is
NIVolume{Float32,3,Array{Int16,3}}
I am not sure why, but it may have to do with the return type of getindex being Float32

Then when writing the nifti it becomes corrupted, as the header always says Float32 datatype

@vsaase
Copy link
Author

vsaase commented Sep 11, 2019

ok, the reason are the lines like
NIVolume{typeof(one(T)*1f0+1f0),N,typeof(raw)}(header, extensions, raw)

which then leads to niupdate corrupting the header

@PetrKryslUCSD
Copy link
Contributor

@vsaase I found essentially the same bug. My solution is different though. I wonder why you solved it the way you did?
What would be the advantage relative to my proposal? (Just wondering if I should chuck mine, in case yours is superior.)

@Tokazama
Copy link
Member

Tokazama commented Dec 9, 2020

I'm tempted to drop the whole distinction between "raw" and actual data type. That would make fixing this straightforward. Thoughts?

@PetrKryslUCSD
Copy link
Contributor

I am not clear on whether this distinction reflects something in the description of the format or not. If it doesn't, it would certainly make things more straightforward if the implementation did not distinguish.

@Tokazama
Copy link
Member

Tokazama commented Dec 9, 2020

Technically there are two values for adding and multiplying the raw data. These serve to minimize the file size if you don't have enough unique values to
need a Float64. I'm pretty sure this was developed with Matlab in mind where it can be painful to manage that manually.

@abrandberg
Copy link

Hi and thank you for a nice package.

I stumbled into this type of problem and found that @PetrKryslUCSD 's solution in #46 solved it for me. If it does not break anything, it would be fantastic if his suggestion could be added to the package.

@Tokazama
Copy link
Member

I'm going to be making a fairly breaking change here soon that will fix this. I'm mostly getting dependencies registered and working.

@ofgulban
Copy link

ofgulban commented Mar 19, 2022

I am having the same problem with integer valued niftis. #57 #46 seems to be related to the same issue. Is there any chance that this will be fixed soon? This issue might be a critical deal-breaker.

By the way, for a workaround, I have used FSL fslmaths utility (here) to cast my int16 nifti to float32:

fslmaths /path/to/my_nifti.nii.gz -mul 1.0 /path/to/my_nifti_float32.nii.gz 

After doing this, niread niwrite works. Might be helpful if someone is looking for a quick workaround where it is not important to preserve the integer nature of the input niftis.

@Tokazama
Copy link
Member

Sorry I haven't had time to fix this. I'm not sure if I'll have time to figure it out for a couple weeks but if someone makes a PR I could take a look

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

5 participants