BIC-MNI / minc-tools

Basic minc-tools from former minc repository
Other
30 stars 25 forks source link

mn2nii failure to produce valid NIfTI files #90

Closed cmadjar closed 4 years ago

cmadjar commented 5 years ago

I ran mnc2nii on multiple MINC files and none of the files are converting properly. I encounter two types of issues:

1. Conversion of 4D datasets (ASL, BOLD and DWI)

All 4D datasets that I tried mnc2nii on were all converted into files with no brain on it. Min and Max values on the NIfTI files were set to 0. Note, the number of volumes in the NIfTI files correspond to the number of volumes in the MINC files but they are all empty images across time.

2. Conversion of 3D datasets (T1W, T2W, GRE T2, multi-echo T2, fieldmap, FLAIR)

All of the 3D NIfTI files are converted but the images are not well oriented (see one example below for the T2W acquisition).

T2W: preventad_5219094_PREBL00_t2w_001_t2w-defaced_001

Information about the mnc2nii version used:

Downloaded with MINC toolkit version 1.9.16 for Ubuntu

program: 2.4.02
libminc: 2.4.02
netcdf : 4.5.0 of Jan 17 2018 17:09:58 $
HDF5   : 1.8.20

More detailed information about the files

I append to this issue a zip file with a folder for each modality that I tried mnc2nii on. Each folder contains:

test_mnc2nii_only_text_files.zip

I did not append the MINC or NIfTI files due to size constraint but can definitely share them with you if you need access to them. Just let me know.

cmadjar commented 5 years ago

Hi MINC developers,

I was wondering if you had a chance to look into this issue?

In the context of open science (CONP, TOSI etc...), we need to be able to convert MINC files into valid NIfTI files (we cannot use dcm2nii since our defacing pipeline runs on MINC files). Since the community of researchers keep asking us to give them NIfTI files, we are stuck and need to have a mnc2nii working versions.

Let me know if you need any additional information. I will be happy to help as much as I can with this issue.

Thanks!

gdevenyi commented 5 years ago

Can you please repeat your tests with minc-toolkit/1.9.17, thanks.

cmadjar commented 5 years ago

Hi Gabriel,

Is there a pre-compiled version available somewhere for 1.9.17? I am trying to compile using the README.md guideline but I get 'Configuring incomplete, errors occurred!' when I run cmake.

History of commands:

git clone https://github.com/BIC-MNI/minc-toolkit-v2.git /opt/minc_github_repo/
cd /opt/minc_github_repo
git checkout release-1.9.17
mkdir build
cd build
cmake .. \
-DCMAKE_BUILD_TYPE:STRING=Release   \
-DCMAKE_INSTALL_PREFIX:PATH=/opt/minc/1.9.17 \
-DMT_BUILD_ABC:BOOL=ON   \
-DMT_BUILD_ANTS:BOOL=ON   \
-DMT_BUILD_C3D:BOOL=ON   \
-DMT_BUILD_ELASTIX:BOOL=ON   \
-DMT_BUILD_IM:BOOL=OFF   \
-DMT_BUILD_ITK_TOOLS:BOOL=ON   \
-DMT_BUILD_LITE:BOOL=OFF   \
-DMT_BUILD_SHARED_LIBS:BOOL=ON   \
-DMT_BUILD_VISUAL_TOOLS:BOOL=ON   \
-DMT_USE_OPENMP:BOOL=ON   \
-DUSE_SYSTEM_FFTW3D:BOOL=OFF   \
-DUSE_SYSTEM_FFTW3F:BOOL=OFF   \
-DUSE_SYSTEM_GLUT:BOOL=OFF   \
-DUSE_SYSTEM_GSL:BOOL=OFF   \
-DUSE_SYSTEM_HDF5:BOOL=OFF   \
-DUSE_SYSTEM_ITK:BOOL=OFF   \
-DUSE_SYSTEM_NETCDF:BOOL=OFF   \
-DUSE_SYSTEM_NIFTI:BOOL=OFF   \
-DUSE_SYSTEM_PCRE:BOOL=OFF   \
-DUSE_SYSTEM_ZLIB:BOOL=OFF 

Log of the cmake command is attached (log_cmake.txt). Looks like it cannot find a lot of the CMakeLists.txt files from what I can gather.

cmake version 3.15.0-rc3

cmadjar commented 5 years ago

nervermind. I just realize I forgot the --recursive option of the git command... Sorry.

cmadjar commented 5 years ago

Hi again,

The issues seem to have been fixed in the latest release of the MINC tools (1.9.17). Thank you for your help!!

Cécile

gdevenyi commented 5 years ago

Sorry I'm a bit late now, but you can find newer packages for all systems at https://packages.bic.mni.mcgill.ca/minc-toolkit/Debian/

I really need to update the BIC webpage...

cmadjar commented 5 years ago

No worries. Thank you for your help!

The link will be very useful for all the upgrades I have to do on multiple servers.

gdevenyi commented 5 years ago

Followup, we have rpms too, the base directory is open: https://packages.bic.mni.mcgill.ca/minc-toolkit/

vfonov commented 5 years ago

You can try with conda too: conda install -c vfonov minc-toolkit-v2 - it doesn't have Display or Register though.

gdevenyi commented 5 years ago

Very nice! @vfonov maybe we can get this into conda-forge?

vfonov commented 5 years ago

my original reason to make it on mainline conda was to make it work with CentOS6 , which is still used in certain places. Recipes are here: https://github.com/vfonov/conda-recipes/

cmadjar commented 5 years ago

Hi again,

So I tried the pre-compile version for Ubuntu 16.04 (minc-toolkit-1.9.17-20190313-Ubuntu_16.04-x86_64.deb) on one of our VM with Ubuntu 16.04 and I basically screwed up the VM :S...

Here is what I ran and the output of the dpkg command:

lorisadmin@preventad-open-dev:/data/not-backed-up$ sudo dpkg -i minc-toolkit-1.9.17-20190313-Ubuntu_16.04-x86_64.deb
[sudo] password for lorisadmin: 
(Reading database ... 127257 files and directories currently installed.)
Preparing to unpack minc-toolkit-1.9.17-20190313-Ubuntu_16.04-x86_64.deb ...
Unpacking minc-toolkit-v2:i386 (1.9.17) over (1.9.16) ...
dpkg: warning: unable to delete old directory '/opt/minc/1.9.16/bin': Directory not empty
dpkg: warning: unable to delete old directory '/opt/minc/1.9.16': Directory not empty
dpkg: dependency problems prevent configuration of minc-toolkit-v2:i386:
 minc-toolkit-v2:i386 depends on perl.
 minc-toolkit-v2:i386 depends on libstdc++6.
 minc-toolkit-v2:i386 depends on libc6.

dpkg: error processing package minc-toolkit-v2:i386 (--install):
 dependency problems - leaving unconfigured
Errors were encountered while processing:
 minc-toolkit-v2:i386

Then I tried to install one of the dependency (sudo apt-get install libc6) which returned:

lorisadmin@preventad-open-dev:/data/not-backed-up$ sudo apt-get install libc6
Reading package lists... Done
Building dependency tree       
Reading state information... Done
libc6 is already the newest version (2.23-0ubuntu11).
You might want to run 'apt-get -f install' to correct these:
The following packages have unmet dependencies:
 minc-toolkit-v2:i386 : Depends: perl:i386 but it is not going to be installed
                        Depends: libstdc++6:i386 but it is not going to be installed
                        Depends: libc6:i386 but it is not going to be installed
E: Unmet dependencies. Try 'apt-get -f install' with no packages (or specify a solution).

So I ran what was suggested apt-get -f install and it uninstalled a lot of packages and screwed up the VM...

I installed MINC tools many times using dpkg and that is the first time that it is not going well. Any idea why?

FYI: ubuntu version: Welcome to Ubuntu 16.04.6 LTS (GNU/Linux 4.4.0-151-generic x86_64)

vfonov commented 5 years ago

looks like somebody packages 1.9.17 32bit as 64bit.

cmadjar commented 5 years ago

:S. That would explain it...

cmadjar commented 5 years ago

Would it be possible to replace the package by one that was created as a 64bit?

gdevenyi commented 5 years ago

You've found a bug in the cpack packaging of minc-toolkit, the 32bit package is getting the same name as the 64bit package and overwriting the 64bit version.

gdevenyi commented 5 years ago

Ref: https://github.com/BIC-MNI/minc-toolkit-v2/issues/86

gdevenyi commented 5 years ago

The packaging has been fixed, rebuilt and re-uploaded

cmadjar commented 5 years ago

Thank you!

cmadjar commented 5 years ago

@gdevenyi It looks like it is trying to into the 1.9.16 directory and not 1.9.17. Is that the intended behaviour?

I don't really want to delete my 1.9.16 directory for now. I was thinking of installing 1.9.17 next to it.

(Reading database ... 127258 files and directories currently installed.)
Preparing to unpack minc-toolkit-1.9.17-20190313-Ubuntu_16.04-x86_64.deb ...
Unpacking minc-toolkit-v2 (1.9.17) over (1.9.16) ...
dpkg: warning: unable to delete old directory '/opt/minc/1.9.16/bin': Directory not empty
dpkg: warning: unable to delete old directory '/opt/minc/1.9.16': Directory not empty
Setting up minc-toolkit-v2 (1.9.17) ...
cmadjar commented 5 years ago

Nevermind... It installed it but failed removing the old instance which is perfect for me.

Thanks again!

cmadjar commented 5 years ago

@gdevenyi It looks like the issue with mnc2nii I reported is present in the package builds but not if we do the install from the Github repo.

cmadjar commented 5 years ago

The 4D datasets are completely empty when converted using mnc2nii. I completely uninstalled 1.9.16 and installed 1.9.17 on the VM I am trying and ended up with the same bug as previously mentioned for the 4D datasets.

Could it be something that happens when creating the pre-built packages? Maybe a missing dependency failing silently?

Thanks

gdevenyi commented 5 years ago

Can you please share some data for me to do testing.

cmadjar commented 5 years ago

You can download an example from the Open PREVENT-AD LORIS instance. I saw you had an account on it. Once logged into that instance, just copy this link: https://openpreventad.loris.ca/api/v0.0.3-dev/candidates/5219094/PREBL00/images/preventad_5219094_PREBL00_bold_001.mnc

(Note any 4D dataset would end up with the same failure but that is the one on which I tested lately)

Let me know if you prefer another way to get the dataset. Thanks!

gdevenyi commented 5 years ago

Can you please describe how you are testing the validity of the NIFTI files and determining there is nothing in them?

cmadjar commented 5 years ago

I open the resulting NIfTI file in MRICRON or fsleyes and can see that the min and max values for the fMRI volumes are 0.

Also, if I try to create a screenshot of the NIfTI file, I get a complaint saying the file is empty, which is indeed the case, there is no image (but the number of volumes is correct though).

When I installed the MINC tool 1.9.17 using the Github repo and make, mnc2nii produces a correct MINC for those 4D datasets. However, if I install 1.9.17 using the pre-built package you just created, I have empty 4D files, just like with the 1.9.16 pre-built package which is why I have a feeling it is something with the pre-built packages that create the issue.

Don't know if that makes sense. Let me know if you need more information.

Thanks!

gdevenyi commented 5 years ago

I have confirmed the issue on my 18.04 MINC-VM.

I am investigating possible sources of issues.

One thing of note is that the mnc2nii in minc-toolkit-v1 shows the same issue, despite being a different version, which points to a build system difference, rather than an issue with the codebase. Right now, I'm leaning towards to cmake weirdness.

gdevenyi commented 5 years ago

changing cmake versions didn't resolve the issue.

gdevenyi commented 5 years ago

Can you please provide some more details on your VM setup? Particularly the build environment. I see for example you had upgraded cmake, have you added any other extra APT sources?

cmadjar commented 5 years ago

Welcome to Ubuntu 16.04.6 LTS (GNU/Linux 4.4.0-154-generic x86_64)

History of commands before I ran make on the git repo:

sudo apt-get install cmake
sudo apt-get install build-essential g++ bc  cmake  bison flex  libx11-dev x11proto-core-dev  libxi6 libxi-dev  libxmu6 libxmu-dev libxmu-headers  libgl1-mesa-dev libglu1-mesa-dev  libjpeg-dev
sudo apt remove cmake
bash /opt/cmake-3.15.0-rc3-Linux-x86_64.sh  # to install latest cmake binary in /opt
sudo ln -s /opt/cmake-3.15.0-rc3-Linux-x86_64/bin/* /usr/local/bin/

For me the apt-get cmake version was 3.5 so I had to remove it and install it from their sh script since I got an error that I needed at least cmake 3.6. I ended up with version 3.15.0-rc3 of cmake on my sandbox.

In case that is helpful, here is the list of the versions of the other dependencies for MINC installed by apt-get:

build-essential/xenial,now 12.1ubuntu2 amd64 [installed]
g++/xenial,now 4:5.3.1-1ubuntu1 amd64 [installed]
bc/xenial,now 1.06.95-9build1 amd64 [installed]
bison/xenial,now 2:3.0.4.dfsg-1 amd64 [installed]
flex/xenial,now 2.6.0-11 amd64 [installed]
libx11-dev/xenial-updates,xenial-security,now 2:1.6.3-1ubuntu2.1 amd64 [installed]
x11proto-core-dev/xenial-updates,xenial-updates,xenial-security,xenial-security,now 7.0.31-1~ubuntu16.04.2 all [installed]
libxi6/xenial,now 2:1.7.6-1 amd64 [installed]
libxi-dev/xenial,now 2:1.7.6-1 amd64 [installed]
libglu1-mesa-dev/xenial,now 9.0.0-2.1 amd64 [installed]
libjpeg-dev/xenial,now 8c-2ubuntu8 amd64 [installed]
libxmu6/xenial,now 2:1.1.2-2 amd64 [installed]
libxmu-dev/xenial,now 2:1.1.2-2 amd64 [installed]
libxmu-headers/xenial,xenial,now 2:1.1.2-2 all [installed]

Aside from the cmake version, I did not have to change anything else.

Hope this helps. Let me know if you need more information.

gdevenyi commented 5 years ago

Was this VM configured in any other way besides these commands? What's its base install? Ubuntu server? Ubuntu desktop? Ubuntu minimal?

cmadjar commented 5 years ago

I have no idea. This is a VM on one of our servers that was created more than 7 years ago and upgraded multiple times... Not really a clean fresh install.

gdevenyi commented 5 years ago

Okay, I'm trying to narrow down differences. My local builds mnc2nii works, and so does yours, but our packages are built in fresh clean docker environments: https://github.com/BIC-MNI/build_packages/blob/master/build_ubuntu_18.04_x64/Dockerfile

cmadjar commented 5 years ago

Looks like the cmake version on the Dockerfile is different than the one I used. Don't know if that could be an issue?

Or maybe the docker is missing another dependency that was not documented yet?

Very wild guesses. I can send the list of packages installed on my VM if that helps, but there are a lot of them...

gdevenyi commented 5 years ago

Yes, I've tried various cmake versions now, it doesn't seem to affect this. If you have anything added to sources.list.d or sources.list that may help.

cmadjar commented 5 years ago

mmm. What a mystery.

my sources.list.d content:


" ============================================================================
" Netrw Directory Listing                                        (netrw v155)
"   /etc/apt/sources.list.d
"   Sorted by      name
"   Sort sequence: [\/]$,\<core\%(\.\d\+\)\=\>,\.h$,\.c$,\.cpp$,\~\=\*$,*,\.o$,\.obj$,\.info$,\.swp$,\.bak$,\~$
"   Quick Help: <F1>:help  -:go up dir  D:delete  R:rename  s:sort-by  x:special
" ==============================================================================
../                                                                                                                                                                         
./
chris-lea-node_js-trusty.list.save
neurodebian.sources.list
neurodebian.sources.list.distUpgrade
neurodebian.sources.list.save
nodesource.list
nodesource.list.distUpgrade
octave-stable-trusty.list
octave-stable-trusty.list.distUpgrade
octave-stable-trusty.list.save
ondrej-php-trusty.list
ondrej-php-trusty.list.distUpgrade
ondrej-php-trusty.list.save

And here is the content of sources.list: sources.list.txt

Not sure what to look for in those files.

gdevenyi commented 5 years ago

At this point I'm stuck. I'm not sure where the issue is originating, maybe @vfonov has an idea.

vfonov commented 5 years ago

maybe environment variables are different?

vfonov commented 5 years ago

I just tried on a DTI scan from 7T, mnc2nii seem to work fine - fsleyes shows images.

gdevenyi commented 5 years ago

Some experimental results: Packages work on centos, fedora and debian, as well as ubuntu 14.04.

Stops working in Xenial and above in Ubuntu

So something in the build system changed in Ubuntu, now investigating what that is.

gdevenyi commented 5 years ago

Also, building locally on Ubuntu bionic results in a broken mnc2nii

vfonov commented 5 years ago

So, on bionic this line: https://github.com/BIC-MNI/libminc/blob/develop/libsrc/value_conversion.c#L404 produces division by zero, on 4D files which results in undefined behaviour. Probably it is handled differently in different versions of compilers.

vfonov commented 5 years ago

Turns out real error is buffer overwrite in https://github.com/BIC-MNI/minc-tools/blob/develop/conversion/nifti1/mnc2nii.c#L145 , here VIO_N_DIMENSIONS=3 , which doesn't work as expected for 4D volumes.

gdevenyi commented 5 years ago

Thanks @vfonov I've checked that out in my package build copies and I'm trying it out.

Can I ask where exactly the overflow was happening? I got the issue and was trying to trace it, but I'm not sure where the code goes wrong. Also, any idea why it worked with earlier versions?

vfonov commented 5 years ago

it worked by magick! Probably memory layout was different , so information about 4th dimension (time) didn't overwrite important data before.

gdevenyi commented 5 years ago

@cmadjar I am uploading packages with @vfonov's fixes to https://packages.bic.mni.mcgill.ca/minc-toolkit/testbuilds/ , can you please give them a try in your environment of choice? Thanks.

cmadjar commented 5 years ago

@gdevenyi It looks like it is fixed. I tested it on an Ubuntu 18.04 VM and the produced 4D datasets look good!

Thank you so much!!

gdevenyi commented 5 years ago

Thanks @cmadjar

@vfonov shall we stamp a new release date and make a new 1.9.17 release or just replace the existing packages?

gdevenyi commented 4 years ago

Fixed