[Nirfast] NIRFASTSLicer_2.0_major_bugs_report+solutions
Alexis Girault
alexis.girault at kitware.com
Fri Jun 22 13:45:52 EDT 2018
Dear Stanislaw,
First of all sorry for the late response, I came back from an extended
break.
Thank you very much for your investigative work and the solutions you
provided. Handling different orientations with Slicer has always been
tricky, and I implemented most of this as a first-year intern while
probably lacking an understanding of the overall issue, so I am really glad
that you were able to bring some additional information to solve that
issue. We hope to build a community around NIRFAST, and getting such
valuable feedback is a great example of how we can benefit from it, so
thanks again!
#1, #2, #3
I currently have little time allocated to working on NIRFAST, however, I'd
be happy to review those changes if you were able to include your fixes in
merge requests on github [1][2]. It seems that your bullet points could
define separate commits since you have already nicely organized the
changes, and the explanations you provided would be great as commit
messages/merge request comments. Is that something you would feel
comfortable with? If not, would you be able to format your comments and
solutions in the issue tracker? [3][4]
[1] https://github.com/nirfast-admin/NIRFASTSlicer/pulls
[2] https://github.com/nirfast-admin/NIRFAST/pulls
[3] https://github.com/nirfast-admin/NIRFASTSlicer/issues
[4] https://github.com/nirfast-admin/NIRFAST/issues
#4
As soon as we have this merged, we can probably release this as a minor fix
version and create then upload packages on github and nirfast.org.
#5
Could you specify what you mean by inactive parameters (maybe in the issue
tracker)? This module is a Matlab module, and it is needed to use
OpenIGTLink and communicate with the Matlab server, but this module is
static as far as inputs and UI goes. The new Create Mesh module is a python
module that allows having a more dynamic UI and error handling, and it
calls the Image2Mesh module internally. We can not remove that module for
that reason, but there might be a way to hide it from the modules list:
feel free to add an issue in the issue tracker if you believe that is
needed. Also, we already have an issue open to support label maps directly
as an input in the Create Mesh module:
https://github.com/nirfast-admin/NIRFASTSlicer/issues/21
Thanks again, looking forward to hearing more from you.
Best,
Alexis Girault
R&D Engineer in Medical Computing
Kitware, Inc.
http://www.kitware.com
(919) 969-6990 x325
On Mon, May 14, 2018 at 6:16 PM Stanislaw Wojtkiewicz <
s.wojtkiewicz at cs.bham.ac.uk> wrote:
> Hi All
>
>
>
> I want to report bugs in NIRFASTSlicer_v.2.0 mesh generation module.
>
> I also deliver solutions (attached).
>
>
>
> Major problems:
>
> - translation of coordinates from patient space to x-y-z space ->
> crushes the module
>
> - unfinished implementation of fiducial coordinates processing
> (since NIRFASTSlicer v1.0) -> fiducials useless
>
>
>
> The ‘Create Mesh’ crush and fiducials problems can be reproduced using my
> head model (generated from MRI with SPM and saved with NIRFASTSlicer2.0).
>
> (Please mail me for the segmentation ‘*.nrrd’ file. It’s only 632KB.
> However, Nirfast mailing list email size limit is 300KB.)
>
> The command:
>
> cd('C:/Program Files/NIRFASTSlicer
> 2.0.0/lib/NIRFASTSlicer-2.0/matlab-modules');
>
>
> Image2Mesh(cli_argsread({'--nirfastDir','D:/PROGRAMS/nirfast8/toolbox','--labelmap','C:/Users/wojtkies/AppData/Local/Temp/NIRFASTSlicer/EICE_vtkMRMLLabelMapVolumeNodeC.nrrd','--fiducials','34.7404,71.4787,25.567','--fiducials','0.909979,77.0329,25.567','--fiducials','-29.8908,69.9639,25.567','--meshdir','D:/DOCUMENTS/GRANTY/BITMAP/Warsaw_meeting_05.2018/TEST/mesh_test','--meshname','nirfast_mesh','--meshtype','Standard','--cell_size','3.5','--cellradiusedge','3','--facet_size','3.5','--facetangle','25','--facetdistance','3'}));
>
>
>
> (Win 10, Matlab R2016a, NIRFASTSLicer2.0 – pure as download, NIRFAST9.0 –
> pure as download)
>
>
>
> DETAILS of the two major problems + other minor/major bugs repairs:
>
>
>
> #1
>
> Coordinates translation
>
> The current implementation (Image2Mesh.m) assumes only one possible
> orientation of object in the patient space. All images are processed as
> oriented 'LPS' . The MRI structure I use was saved in
> 'right-anterior-superior' (‘RAS’) orientation.
>
> My spacedir matrix (rotation/scale matrix from patient to xyz space) is
> as follows (I have 1mm voxels spacing, thus ones in the matrix):
>
> 0 0 1
>
> -1 0 0
>
> 0 1 0
>
> However, the code assumes nonzero diagonal values (as it is true for ‘LPS’
> patient space orientations). Thus, the following lines:
>
> % param.TransformMatrix(1,1) = param.TransformMatrix(1,1) /
> abs(spacedir(1,1));
>
> % param.TransformMatrix(2,2) = param.TransformMatrix(2,2) /
> abs(spacedir(2,2));
>
> % param.TransformMatrix(3,3) = param.TransformMatrix(3,3) /
> abs(spacedir(3,3));
>
> produce NaNs at the diagonal of the transformation matrix. We divide 0 by
> 0. This leads to a big crush.
>
>
>
> I think, that the best solution is to use affine transform (param.TransformMatrix
> = img.ijkToLpsTransform;). This affine matrix is called ‘ijkToLpsTransform’
> but Slicer puts there correct values no matter what is the patient space
> orientation (i.e. it does not have to be LPS).
>
> The rotation and translation of the model are handled separately despite a
> ready to use affine matrix. Is there any reason to not trust this matrix? I
> checked it for different DICOMS (a neck and heads carried out by different
> hospitals) and this affine transform matrix was always good (for different
> patient orientations!). I am not sure if I fully understand the terminology
> LPS etc. and what exactly is passed from Slicer (I just tried to reverse
> engineer this problem). However, it looks like the proposed solution works.
>
>
>
> Briefly, my changes to the Image2Mesh.m file:
>
> - the affine transform instead of rotation and translation
>
> The rotation was carried out inside ‘Image2Mesh.m’ file, the translation
> was carried out inside ‘RunCGALMeshGenerator.m’ file. The affine transform
> does all at once. I have moved the affine operation into ‘Image2Mesh.m’ as
> this file is closely related to the Slicer which generates the affine
> transformation matrix. And it might be more clear to keep the
> transformation as close as possible to the transformation matrix.
>
> - with the affine transform, there is no need of the
> hard/hand-coded repairs of the rotation matrix (is this to match the
> fiducials coordinates? I do not understand this rotations around 0X and 0Y
> axes.):
>
> % param.TransformMatrix(1,1) = -1 * param.TransformMatrix(1,1);
>
> % param.TransformMatrix(2,2) = -1 * param.TransformMatrix(2,2);
>
> - removed redundancies
>
> - code polishing, etc.
>
>
>
> The fix requires changes in two files: RunCGALMeshGenerator.m (NIRFAST
> package) and Image2Mesh.m (Slicer package). Fixed files attached.
>
>
>
> #2
>
> Unfinished fiducials business
>
> Fiducials coordinates are translated internally by the Slicer (by the same
> vector as in the affine transform in #1). However, they are not rotated.
>
> Fiducials look like they are always in the same patient space, no matter
> what is the data patient orientation. Thus, to convert the fiducials to the
> x-y-z coordinates a following rotation matrix should be used:
>
> -1 0 0
>
> 0 -1 0
>
> 0 0 1
>
>
>
> I have added the following to the ‘Image2Mesh.m’ file:
>
> sdcoords = ([-1 0 0;0 -1 0; 0 0 1]*sdcoords')';
>
> This and affine transform of the model brings the model and fiducials to
> the same space.
>
>
>
> BTW: The fiducials coordinates passed to the ‘Image2Mesh.m’ function and
> saved as a *.csv file are different….
>
>
>
> #3
>
> Reading fiducials fails if there is only one fiducial.
>
> sdcoords = cell2mat(fiducials)';
>
> For one fiducial, the ‘fiducials’ is not a cell and the command fails
> returning error.
>
>
>
> Problem solved in the attached file.
>
>
>
> #4
>
> Can you repair/swap RunCGALMeshGenerator.m and Image2Mesh.m files in the
> packages to download?
>
> I am going to run a training session ‘from MRI to FEM model’ for PhD
> students soon (in 2 weeks). The very first thing I will have to do is to
> ask everybody to repair tools by replacing files here and there (or use my
> Matlab meshing implementation directly on SPM files, which I developed to
> test the errors).
>
> I have to work on DICOM files that confuses NIRFASTSlicer. So, I have no
> choice but distribute NIRFASTSlicer with my repaired files.
>
>
>
> #5
>
> ‘Image2Mesh (Matlab)’ module? A testing leftover? It has inactive
> settings/parameters. These parameters are loaded from the ‘Create Mesh’
> module. There is no easy way to guess that. I propose to delete this
> redundant module. Or repair the inactive options and leave it as backward
> compatibility for meshing ‘LabelMaps’ not ‘Segmentations’ as in the new
> ‘Create Mesh’ module. Or add option of meshing a ‘LabelMap’ to the ‘Create
> Mesh’ module?
>
>
>
>
>
> Regards
>
> Stanislaw Wojtkiewicz, PhD
>
>
>
> ---
>
> School of Computer Science
>
> University of Birmingham
>
> B15 2TT
>
>
>
>
>
>
> <https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=emailclient> Virus-free.
> www.avast.com
> <https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=emailclient>
> <#m_-9174478099627014613_DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
> _______________________________________________
> Nirfast mailing list
> Nirfast at public.kitware.com
> https://public.kitware.com/mailman/listinfo/nirfast
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://public.kitware.com/pipermail/nirfast/attachments/20180622/3cb58e40/attachment-0001.html>
More information about the Nirfast
mailing list