MINC2.0 IO Support for ITK

Please use this identifier to cite or link to this publication: http://hdl.handle.net/1926/147
MINC2.0 IO support for ITK.
Data
minus 5 Files (8Mb)
Code
There is no code review at this time.

Reviews
minus Review of MINC2 IO. by Richard Beare on 07-03-2006 for revision #1
starstarstarstarstar expertise: 4 sensitivity: 5
yellow
Summary:
This paper introduces IO support for MINC 2.0 images. MINC, from the Montreal Neurological Institute, is a format derived from netcdf and hdf5, and there is a range of valuable tools that will be much more accessible if the file format is usable from ITK.

Evidence:
The authors provides example data files and sample code. There is no confirmation that the data is correct once loaded - This could be done using the other minc tools, or ImageCompare.

Open Science:
The work adheres to Open Science principles.

Reproducibility:
I was successful in reproducing the work.

Use of Open Source Software:
The minc libraries are open source and use other open source tools.

Open Source Contributions:
Yes, but see suggestions for future work.

Suggestions for future work:
While this contribution is a very good start, it isn't fully integrated into the toolkit. It would be a great benefit if it was. Other reviewers have recommended ways in which the end user can carry out this integration, and I had much the same experience. I would strongly encourage ITK developers to look at including support for MINC2 directly in ITK, because there are a variety of valuable applications from MNI that work with minc format images.

I discovered incompatibility problems between the code included in this project and various versions of the minc2 libraries. It appears that the most recent version of minc2 changed the API slightly, resulting in compilation errors for this project. Earlier versions had problems with some releases of hdf5. I hope that these are issues that will get sorted out with time.


Additional Comments:
A good start for what should become a very valuable contribution.
minus Review on linux debian system for the MINC2 integration in ITK. by Mathieu Malaterre on 02-03-2006 for revision #1
starstarstarstarstar expertise: 4 sensitivity: 5
yellow
Summary:
This is submission to add support for MINC2 file format for the Insight toolkit.

Evidence:
Author provide the standard mechanism in ITK to read and write IO file (lacking the factory, but author seems to have it).

Open Science:
The work is reproducible.

Reproducibility:
I did download, compile and run the code (test example + demo file). Nothing is described to let people know about expected result.

#1. The code is missing a CMakeLists.txt file. Here is the one I used:
---------------------------------------------------------------------------------------------------
PROJECT(ITKMINC)
FIND_PACKAGE(ITK REQUIRED)
INCLUDE(${ITK_USE_FILE})
INCLUDE(${ITKMINC_SOURCE_DIR}/FindMINC2.cmake)
INCLUDE_DIRECTORIES(${MINC2_INCLUDE_DIR})
ADD_EXECUTABLE(itkminc itkMINC2ImageIOTest.cxx itkMINC2ImageIO.cxx)
TARGET_LINK_LIBRARIES(itkminc ITKIO ${MINC2_LIBRARIES} hdf5 netcdf)
---------------------------------------------------------------------------------------------------

and I also wrote a FindMINC2.cmake since the library is not so standard (very new)

---------------------------------------------------------------------------------------------------
FIND_PATH(MINC2_INCLUDE_DIR minc2.h
/usr/include
/usr/local/include
)
FIND_LIBRARY(MINC2_LIBRARIES minc2
/usr/lib
/usr/local/lib
)
---------------------------------------------------------------------------------------------------

Then on a regular debian system you also need to install the following package:
netcdfg-dev
libhdf5-serial-dev
Recommended package are:
minc-tools
hdf5-tools
(although minc-tools only support minc1 file).

I ran into multiple compilation issues, first one is none const correctness which is a big issue for ITK. I made some change to the code to replacer 'char*' with std::string and char** with std::vector
This affect: GetDimensionOrder / SetDimensionOrder
Furthermore m_DimensionOrder was leaking (not destroyed in destructor) replacing it with std::string, ensure both const correctness and prevent memory leak. Same problem with m_DimensionName (never deleted).
Then the test file try to includes: itkMINC2ImageIOFactory.h which not available, removing the guilty line fix the compilation
There is problem with #define MINC2_MAXDIM 10 what does it refers to ? This is only in the cxx, and not header, this cannot be user driven. More comment should be made about this #define
Even after all those patch the code compiles with lots of warnings:

/tmp/minc/itkMINC2ImageIO.cxx: In member function 'virtual void itk::MINC2ImageIO::Read(void*)':
/tmp/minc/itkMINC2ImageIO.cxx:111: warning: comparison between signed and unsigned integer expressions
/tmp/minc/itkMINC2ImageIO.cxx: In member function 'virtual void itk::MINC2ImageIO::ReadImageInformation()':
/tmp/minc/itkMINC2ImageIO.cxx:253: warning: comparison between signed and unsigned integer expressions
/tmp/minc/itkMINC2ImageIO.cxx:354: warning: comparison between signed and unsigned integer expressions
/tmp/minc/itkMINC2ImageIO.cxx: In member function 'virtual void itk::MINC2ImageIO::Write(const void*)':
/tmp/minc/itkMINC2ImageIO.cxx:474: warning: comparison between signed and unsigned integer expressions
/tmp/minc/itkMINC2ImageIO.cxx:479: warning: unused variable 'dimseparation'
/tmp/minc/itkMINC2ImageIO.cxx:480: warning: unused variable 'dimstart'
/tmp/minc/itkMINC2ImageIO.cxx:581: warning: comparison between signed and unsigned integer expressions
/tmp/minc/itkMINC2ImageIO.cxx:538: warning: unused variable 'minval'
/tmp/minc/itkMINC2ImageIO.cxx:539: warning: unused variable 'maxval'
/tmp/minc/itkMINC2ImageIO.cxx:541: warning: unused variable 'nChannels'
/tmp/minc/itkMINC2ImageIO.cxx: At global scope:
/tmp/minc/itkMINC2ImageIO.cxx:593: warning: unused parameter 'fileName'
/tmp/minc/itkMINC2ImageIO.cxx:593: warning: unused parameter 'buffer'
/tmp/minc/itkMINC2ImageIO.cxx: In member function 'void itk::MINC2ImageIO::SetSliceScalingFromLocalScaling(mivolume*)':
/tmp/minc/itkMINC2ImageIO.cxx:631: warning: comparison between signed and unsigned integer expressions
/tmp/minc/itkMINC2ImageIO.cxx:636: warning: comparison between signed and unsigned integer expressions

Running through valgrind lead to the following memory leak:
Running the example throught valgrind lead to the following problems:

==22892== Conditional jump or move depends on uninitialised value(s)
==22892== at 0x80AB747: _miget_volume_class (volume.c:885)
==22892== by 0x80ABFBB: miopen_volume (volume.c:1061)
==22892== by 0x809A392: itk::MINC2ImageIO::ReadImageInformation() (itkMINC2ImageIO.cxx:190)
==22892== by 0x80945BE: itk::ImageFileReader, itk::DefaultConvertPixelTraits >::GenerateOutputInformation() (itkImageFileReader.txx:138)
==22892== by 0x475BF12: itk::ProcessObject::UpdateOutputInformation() (itkProcessObject.cxx:690)
==22892== by 0x8083FFD: itk::ImageBase<3>::UpdateOutputInformation() (itkImageBase.txx:174)
==22892== by 0x471692F: itk::DataObject::Update() (itkDataObject.cxx:342)
==22892== by 0x475BFCC: itk::ProcessObject::Update() (itkProcessObject.cxx:554)
==22892== by 0x806E804: main (itkMINC2ImageIOTest.cxx:71)

Inspecting result, using the micnc tools still gives some errors are reported:
$ mincstats LF2.mnc
*** /tmp/minc-install/bin/mincstats - reported max (6) doesn't equal header (255)
$ mincstats test5.mnc
*** /tmp/minc-install/bin/mincstats - reported max (0.0015259) doesn't equal header (1)


Use of Open Source Software:
The provided code will allow the open source ITK package to read the file format of the open source MINC tools. It is entirely open source based.

Open Source Contributions:
Author did provide part of the code they use. More code need to be sent for full integration into ITK. See Reproducibility for problems at compile time.

Code Quality:
On overall the code did not respect ITK coding style ( identation, comment, use of cerr instead of itkDebugMacro). there are at least two functions not used:
ReadVolume and WriteSlice. There are some commented code at toplevel of cxx file.

The code itself might also be leaking since I can see a:
midimhandle_t *hdims = new midimhandle_t[m_NDims];
but nowhere can I find a delete[]

And a minor problem is the use of const char pointer instead of cstring:
const char *vname = "vector_dimension";
should be
const char vname[] = "vector_dimension";

Last point: the code is not PIMPLed. This means that ITK will have to include netcdf, hdf and minc header file every time one include the itkMINC2 header.

Suggestions for future work:
The code should be reviewed by author for next submission. More test should be written to. In particular it should not segfault on minc1 file. Commented code should go away, unused code too.
Also submission to Insight Journal require the use of template. As far as I understand this is not the case:
http://www.itk.org/Wiki/ITK_Procedure_for_Contributing_New_Classes_and_Algorithms#How_to_Prepare_a_Submission_to_the_Insight_Journal

Additional Comments:
As usual file format are the entry point to ITK world and therefore is always very welcome.
As a side note the external libraries involve are huge, therefore I would delay until some agreement is made for which library actually goes into ITK.
minus Reviewed by Rupert Brooks on 12-09-2005 for revision #1
starstarstarstarstar expertise: 3 sensitivity: 5
yellow
UPDATE:
After corresponding with the author, i would adjust my comments somewhat. Specifically, the reader was never intended to read minc version 1 files, so the fact that it doesn't is not really a problem. Some problems that i was having were due to a faulty build of the minctools i was using, rather than anything to do with this code particularly.

For those who might like to use this module, i would point out two things:

  1. You can convert your old minc 1 files, such as brainweb files, to minc 2 using the mincconvert utility found on the website listed in the paper. Note that mincconvert does not automatically get installed with the release that I used (2.0.09), you have to manually install it, this should be fixed in the next release The command line to use is:

    mincconvert -2 input.mnc output.mnc

  2. Secondly, when i built the minc tools, i ran into problems using the HDF library version 1.6.5, but it works fine with 1.6.4. I am not sure why, but if you experience HDF errors when trying to convert minc files, this may be the source of the trouble.



Summary:
The paper presents code to enable ITK to read and write the MINC2 file format.

Evidence:
The authors provide a small test program which manipulates one test dataset that is provided. The test strategy seems to be visual - the results of the program are inspected for correctness by eye - rather than numerical.

Open Science:
The paper adheres to the principles of open science. The work is reproducible.

Reproducibility:
I downloaded, compiled and ran the code. In that sense I reproduced the work. However the test strategy may not be adequate to fully test the code.

As an example, I tried to use the code to read my own data, but I had difficulty. I was only able to read the data provided with the code. In particular, i was not able to read data from MNI Brainweb, which would probably be a major use of this module. I got the following error in all cases:

rbrook@spender{IO}../../test/itkMINC2ImageIOTest ~/t1_icbm_normal_1mm_pn3_rf20.mnc
This is a test for MINC2!!
exception in file reader
itk::ERROR: MINC2ImageIO(0x85a9b80): Unknown component type: 0
Unknown

I am not sure if this problem was due to my data being in MINC1 format, or other reasons. The MINC2 library is supposed to be backwards compatible with MINC1, but it may be the case that the module presented here does not support reading minc1. I did not pursue this in great detail at this time.

Use of Open Source Software:
The provided code will allow the open source ITK package to read the file format of the open source MINC tools. It is entirely open source based.

Open Source Contributions:
Source code is provided, however, it does not compile without editing. The line

#include "itkMINC2ImageIOFactory.h"

must be commented out of itkMINC2ImageIOTest.cxx for it to compile. Furthermore, the writing in the paper implies that other code may exist that was not provided. Specifically, they mention that the module was added to ITK as an advanced option, but no CMAKE was provided. Also, the use of the pluggable factories was discussed - but an itkMINC2ImageIOFactory class was not provided. The commented line above implies that it exists somewhere.

Additional documentation on building the needed external libraries may be helpful. To assist other reviewers, i give some tips here - these worked for me on RedHat8 linux, there may be other and better solutions.

  1. In addition to the minc2, hdf and netcdf libraries, i found that the szip library was also required. Its true that its required for HDF, not for this code particularly, but its still useful to know.
  2. When building and installing these libraries, build and install them in this order: netcdf, szip, hdf, minc2.
  3. When linking the provided code, i found that the following link order worked
    <.o files> ... other stuff.... -lminc2 -lnetcdf -lhdf5 -lsz -lz


Code Quality:
The authors name and affiliation only appears in the .h file, not at the top of the .cxx file. The test file does not seem to meet the ITK testing standard of testing every method thoroughly. Beyond that, I did not read the code in detail.

Suggestions for future work:
I think that a more thorough test strategy would be needed to verify the code. As an example, minc is capable of storing data internally in any orientation and order (that is xyz, zyx, row major, column major, etc). I have seen other minc reading code work (using the minc1.4 library) on some orders and not others, so a thorough testing is required. Same applies for different numbers of dimensions, and different types of data (byte, int, etc).

Adding the Factory classes would be very useful.

A CMAKE build script, or instructions for modding the ITK CMakeList would also be helpful.

Is the code intended to read MINC version 1 files? The minc2 library should be capable of it and this would be useful.

Additional Comments:
I think that this is an extremely useful contribution. Once tested and shown to be reliable, i would probably use this daily in my work. The MINC tools provide a valuable set of open source medical imaging tools, complementary to ITK. Furthermore, there is much data available in MINC, and the conversion process into a format that ITK can read and vice versa is time-consuming and risks losing data.

With regard to the paper itself, it would be important to add the author and date somewhere. It is not obvious how to contact the author. Style and grammar wise there are some issues, but as the main contribution is the code, i consider this relatively less important.
Add a new review
Quick Comments


Resources
backyellow
Download All

Statistics more
backyellow
Global rating: starstarstarstarstar
Review rating: starstarstarstarstar [review]
Code rating:
Paper Quality: plus minus

Information more
backyellow
Categories: IO
Keywords: software, programming
Toolkits: ITK, CMake
Export citation:

Share
backyellow
Share

Linked Publications more
backyellow
RANSAC Plane Fitting for VTK RANSAC Plane Fitting for VTK
by Doria D.
Diffeomorphic Demons Using ITK's Finite Difference Solver Hierarchy Diffeomorphic Demons Using ITK's Finite Difference Solver Hierarchy
by Vercauteren T., Pennec X., Perchant A., Ayache N.

View license
Loading license...

Send a message to the author
main_flat
Powered by Midas