Comments (10)
If this solution works for everybody I'll get the release out today
from nifti.jl.
from nifti.jl.
Experiencing the same error. Current workaround is manually scaling the data as follows:
scaled_data = ni.raw .* ni.header.scl_slope .+ ni.header.scl_inter
Any updates on fixing this @Tokazama ?
from nifti.jl.
Is anyone depending on rescaling? It's really just a bad idea to have as part of the standard and I think it causes a lot of unintuitive problems for users. I could just make it an opt-in parameter when loading.
from nifti.jl.
It isn't important for the magnitude really but it's pretty essential for phase (i.e phase in Integers is nonsensical). For this case,either the researcher must do the scaling or the scaling is handled properly by NIfTI.jl. It might make sense to have an option to read the NIVolume in as a specific type (i.e Float32, Float64, etc...), and then an InexactError would be at least informative.
from nifti.jl.
Yeah, and the use of anything other than 32 and 64 bit values will definitely be problematic for anyone trying to get performance for any serious computational work.
I'm assuming the original intent behind automatically scaling values was to help researchers who didn't know anything about data types but wanted to save memory when storing data. The natural way to handle this in Julia is to just to do Float32.(load(file)))
and if you are going to do multiple sessions with that file you might want to just write over it with the converted data.
I'm not sure if there's a simpler way to support that in the load argument though. What does your typical workflow look like?
from nifti.jl.
I found the bug. It's a bug in the getindex method specific to the NIVolume object. The base getindex() method returns an array which is by default the same type as the elements passed into it, in our case f.raw, which can be a different type than f.header.scl_inter or f.header.scl_slope. See line 285 in src/NIfTI.jl. Either this line should be changed to avoid the type incompatibility, or (in the interest of simplicity) the functionality of indexing into the NIVolume should be removed and we should promote and document rescaling of the values independently of the data loading.
from nifti.jl.
The problem that we have right now seem quite common to me. occurring whenever you have a Int16 NIfTI file with scaling. For a quick data analysis script, it is quite ok to do external scaling, although confusing for newcomers to be greeted with an error message. For processing pipelines, this case has to be always considered and specially handled, or rather the implicit scaling should never be used. This is quite inconvenient and error prone at the moment.
In this pull request, I had a go at this with a simple solution, by decoupling the NIVolume type from the type of the raw volume. The only place to adjust was the niupdate function to set the type in the nifti header to the eltype
of the raw field instead of the NIVolume type.
I think this change wouldn't worsen any workflow. The main difference would be that Int16 files, when accessed via indexing, i.e. ni_image[:,:,:] return a Float32 image instead of Int16. In my opinion this makes most sense, since in the general case there is a scaling factor that is a float as well. If the underlying datatype is required, it makes sense to access it via ni_image.raw.
The biggest disadvantage would probably be that it might be a breaking change, it could either be 0.6 or 1.0, since it has been stable for quite some time?
from nifti.jl.
Thanks for merging!
from nifti.jl.
@alexjaffray Does this solution also seem fine to you?
@Tokazama Can we go ahead with releasing this as version 0.6?
from nifti.jl.
Related Issues (20)
- feature request/change HOT 1
- Why are method2 and method3 mutually exclusive? HOT 1
- Is this still active? HOT 1
- bug with Integer valued niftis HOT 9
- Transfer to the JuliaHealth organization? HOT 17
- Feature request: Reading and writing Bool datatype HOT 2
- TagBot trigger issue HOT 21
- Maybe a function is needed here to just read the header information HOT 3
- Confused about constructor HOT 1
- ArgumentError: invalid open mode: w9
- Bug in getaffine function HOT 1
- Reorient based on RAS/LPI HOT 2
- Files written by niwrite are corrupted HOT 8
- Consider making it difficult to make (u)int64 NIfTI images HOT 2
- NIfTI-2 support HOT 3
- mean of 3d nifti does not match calc by fslstats and 3dBrickStat HOT 2
- is this still active? HOT 1
- Missing fields for Freq_dim, Phase_dim and slice_dim within the nifti header after reading a nifti file. HOT 2
- Adding support for sform and qform matrices as fields within the loaded nifti file header. HOT 8
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from nifti.jl.