<div dir="ltr">Dear Stanislaw,<div><br></div><div>First of all sorry for the late response, I came back from an extended break.</div><div><br></div><div>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!</div><div><br></div><div>#1, #2, #3</div><div>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] </div><div><br></div><div>[1] <a href="https://github.com/nirfast-admin/NIRFASTSlicer/pulls">https://github.com/nirfast-admin/NIRFASTSlicer/pulls</a></div><div>[2] <a href="https://github.com/nirfast-admin/NIRFAST/pulls">https://github.com/nirfast-admin/NIRFAST/pulls</a></div><div>[3] <a href="https://github.com/nirfast-admin/NIRFASTSlicer/issues">https://github.com/nirfast-admin/NIRFASTSlicer/issues</a></div><div>[4] <a href="https://github.com/nirfast-admin/NIRFAST/issues">https://github.com/nirfast-admin/NIRFAST/issues</a></div><div><br></div><div>#4</div><div>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 <a href="http://nirfast.org">nirfast.org</a>.</div><div><br></div><div>#5</div><div>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: <a href="https://github.com/nirfast-admin/NIRFASTSlicer/issues/21">https://github.com/nirfast-admin/NIRFASTSlicer/issues/21</a></div><div><br></div><div>Thanks again, looking forward to hearing more from you.</div><div><br></div><div>Best,</div><div><br></div><div><div><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><span style="color:rgb(136,136,136);font-size:12.8px">Alexis Girault</span><br style="color:rgb(136,136,136);font-size:12.8px"><span style="color:rgb(136,136,136);font-size:12.8px">R&D Engineer in Medical Computing</span><br style="color:rgb(136,136,136);font-size:12.8px"><span style="color:rgb(136,136,136);font-size:12.8px">Kitware, Inc.</span><br style="color:rgb(136,136,136);font-size:12.8px"><br style="color:rgb(136,136,136);font-size:12.8px"><a href="http://www.kitware.com/" rel="noreferrer" style="color:rgb(17,85,204);font-size:12.8px" target="_blank">http://www.kitware.com</a><br style="color:rgb(136,136,136);font-size:12.8px"><font color="#999999"><a target="_blank"><span style="font-size:12.8px">(919) 969-6990 x3</span>25</a></font><br></div></div></div></div></div></div></div></div></div><br></div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, May 14, 2018 at 6:16 PM Stanislaw Wojtkiewicz <<a href="mailto:s.wojtkiewicz@cs.bham.ac.uk">s.wojtkiewicz@cs.bham.ac.uk</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="EN-GB" link="blue" vlink="purple"><div class="m_-9174478099627014613WordSection1"><p class="MsoNormal">Hi All<u></u><u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal">I want to report bugs in NIRFASTSlicer_v.2.0 mesh generation module.<u></u><u></u></p><p class="MsoNormal">I also deliver solutions (attached).<u></u><u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal">Major problems: <u></u><u></u></p><p class="m_-9174478099627014613MsoListParagraph"><u></u><span>-<span style="font:7.0pt "Times New Roman""> </span></span><u></u>translation of coordinates from patient space to x-y-z space -> crushes the module<u></u><u></u></p><p class="m_-9174478099627014613MsoListParagraph"><u></u><span>-<span style="font:7.0pt "Times New Roman""> </span></span><u></u>unfinished implementation of fiducial coordinates processing (since NIRFASTSlicer v1.0) -> fiducials useless<u></u><u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal">The ‘Create Mesh’ crush and fiducials problems can be reproduced using my head model (generated from MRI with SPM and saved with NIRFASTSlicer2.0).<u></u><u></u></p><p class="MsoNormal">(Please mail me for the segmentation ‘*.nrrd’ file. It’s only 632KB. However, Nirfast mailing list email size limit is 300KB.)<u></u><u></u></p><p class="MsoNormal">The command:<u></u><u></u></p><p class="MsoNormal">cd('C:/Program Files/NIRFASTSlicer 2.0.0/lib/NIRFASTSlicer-2.0/matlab-modules');<u></u><u></u></p><p class="MsoNormal">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'}));<u></u><u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal">(Win 10, Matlab R2016a, NIRFASTSLicer2.0 – pure as download, NIRFAST9.0 – pure as download)<u></u><u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal">DETAILS of the two major problems + other minor/major bugs repairs:<u></u><u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal">#1<u></u><u></u></p><p class="MsoNormal">Coordinates translation<u></u><u></u></p><p class="MsoNormal">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.<u></u><u></u></p><p class="MsoNormal" style="text-autospace:none">My <span style="font-size:13.0pt;font-family:"Courier New";color:black">spacedir</span> matrix (rotation/scale matrix from patient to xyz space) is as follows (I have 1mm voxels spacing, thus ones in the matrix):<u></u><u></u></p><p class="MsoNormal" style="text-autospace:none"><span style="font-size:12.0pt;font-family:"Courier New""> 0 0 1<u></u><u></u></span></p><p class="MsoNormal" style="text-autospace:none"><span style="font-size:12.0pt;font-family:"Courier New""> -1 0 0<u></u><u></u></span></p><p class="MsoNormal" style="text-autospace:none"><span style="font-size:12.0pt;font-family:"Courier New""> 0 1 0<u></u><u></u></span></p><p class="MsoNormal">However, the code assumes nonzero diagonal values (as it is true for ‘LPS’ patient space orientations). Thus, the following lines:<u></u><u></u></p><p class="MsoNormal" style="text-autospace:none"><span style="font-size:13.0pt;font-family:"Courier New";color:forestgreen">% param.TransformMatrix(1,1) = param.TransformMatrix(1,1) / abs(spacedir(1,1));</span><span style="font-size:12.0pt;font-family:"Courier New""><u></u><u></u></span></p><p class="MsoNormal" style="text-autospace:none"><span style="font-size:13.0pt;font-family:"Courier New";color:forestgreen">% param.TransformMatrix(2,2) = param.TransformMatrix(2,2) / abs(spacedir(2,2));</span><span style="font-size:12.0pt;font-family:"Courier New""><u></u><u></u></span></p><p class="MsoNormal" style="text-autospace:none"><span style="font-size:13.0pt;font-family:"Courier New";color:forestgreen">% param.TransformMatrix(3,3) = param.TransformMatrix(3,3) / abs(spacedir(3,3));</span><span style="font-size:12.0pt;font-family:"Courier New""><u></u><u></u></span></p><p class="MsoNormal">produce NaNs at the diagonal of the transformation matrix. We divide 0 by 0. This leads to a big crush.<u></u><u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal" style="text-autospace:none">I think, that the best solution is to use affine transform (<span style="font-size:13.0pt;font-family:"Courier New";color:black">param.TransformMatrix = img.ijkToLpsTransform;</span>). This affine matrix is called ‘<span style="font-size:13.0pt;font-family:"Courier New";color:black">ijkToLpsTransform</span>’ but Slicer puts there correct values no matter what is the patient space orientation (i.e. it does not have to be LPS). <u></u><u></u></p><p class="MsoNormal" style="text-autospace:none">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.<u></u><u></u></p><p class="MsoNormal" style="text-autospace:none"><u></u> <u></u></p><p class="MsoNormal" style="text-autospace:none">Briefly, my changes to the Image2Mesh.m file:<u></u><u></u></p><p class="m_-9174478099627014613MsoListParagraph" style="text-autospace:none"><u></u><span>-<span style="font:7.0pt "Times New Roman""> </span></span><u></u>the affine transform instead of rotation and translation<u></u><u></u></p><p class="m_-9174478099627014613MsoListParagraph" style="text-autospace:none">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.<u></u><u></u></p><p class="m_-9174478099627014613MsoListParagraph" style="text-autospace:none"><u></u><span>-<span style="font:7.0pt "Times New Roman""> </span></span><u></u>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.):<u></u><u></u></p><p class="m_-9174478099627014613MsoListParagraph" style="text-autospace:none"><span style="font-size:13.0pt;font-family:"Courier New";color:forestgreen">% param.TransformMatrix(1,1) = -1 * param.TransformMatrix(1,1);</span><span style="font-size:12.0pt;font-family:"Courier New""><u></u><u></u></span></p><p class="m_-9174478099627014613MsoListParagraph" style="text-autospace:none"><span style="font-size:13.0pt;font-family:"Courier New";color:forestgreen">% param.TransformMatrix(2,2) = -1 * param.TransformMatrix(2,2);</span><span style="font-size:12.0pt;font-family:"Courier New""><u></u><u></u></span></p><p class="m_-9174478099627014613MsoListParagraph" style="text-autospace:none"><u></u><span>-<span style="font:7.0pt "Times New Roman""> </span></span><u></u>removed redundancies<u></u><u></u></p><p class="m_-9174478099627014613MsoListParagraph" style="text-autospace:none"><u></u><span>-<span style="font:7.0pt "Times New Roman""> </span></span><u></u>code polishing, etc.<u></u><u></u></p><p class="MsoNormal" style="text-autospace:none"><u></u> <u></u></p><p class="MsoNormal" style="text-autospace:none">The fix requires changes in two files: RunCGALMeshGenerator.m (NIRFAST package) and Image2Mesh.m (Slicer package). Fixed files attached.<u></u><u></u></p><p class="MsoNormal" style="text-autospace:none"><span style="font-size:12.0pt;font-family:"Courier New""><u></u> <u></u></span></p><p class="MsoNormal" style="text-autospace:none"><span style="font-size:12.0pt;font-family:"Courier New"">#2<u></u><u></u></span></p><p class="MsoNormal">Unfinished fiducials business<u></u><u></u></p><p class="MsoNormal">Fiducials coordinates are translated internally by the Slicer (by the same vector as in the affine transform in #1). However, they are not rotated.<u></u><u></u></p><p class="MsoNormal">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:<u></u><u></u></p><p class="MsoNormal"> -1 0 0<u></u><u></u></p><p class="MsoNormal"> 0 -1 0<u></u><u></u></p><p class="MsoNormal"> 0 0 1<u></u><u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal">I have added the following to the ‘Image2Mesh.m’ file:<u></u><u></u></p><p class="MsoNormal" style="text-autospace:none"><span style="font-size:13.0pt;font-family:"Courier New";color:black">sdcoords = ([-1 0 0;0 -1 0; 0 0 1]*sdcoords')';</span><span style="font-size:12.0pt;font-family:"Courier New""><u></u><u></u></span></p><p class="MsoNormal">This and affine transform of the model brings the model and fiducials to the same space.<u></u><u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal">BTW: The fiducials coordinates passed to the ‘Image2Mesh.m’ function and saved as a *.csv file are different….<u></u><u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal">#3<u></u><u></u></p><p class="MsoNormal">Reading fiducials fails if there is only one fiducial.<u></u><u></u></p><p class="MsoNormal" style="text-autospace:none"><span style="font-size:13.0pt;font-family:"Courier New";color:black">sdcoords = cell2mat(fiducials)';</span><span style="font-size:12.0pt;font-family:"Courier New""><u></u><u></u></span></p><p class="MsoNormal">For one fiducial, the ‘fiducials’ is not a cell and the command fails returning error. <u></u><u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal">Problem solved in the attached file.<u></u><u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal">#4<u></u><u></u></p><p class="MsoNormal">Can you repair/swap RunCGALMeshGenerator.m and Image2Mesh.m files in the packages to download?<u></u><u></u></p><p class="MsoNormal">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).<u></u><u></u></p><p class="MsoNormal">I have to work on DICOM files that confuses NIRFASTSlicer. So, I have no choice but distribute NIRFASTSlicer with my repaired files.<u></u><u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal">#5<u></u><u></u></p><p class="MsoNormal">‘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?<u></u><u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal"><span lang="EN-US" style="font-family:Consolas">Regards<u></u><u></u></span></p><p class="MsoNormal"><span lang="EN-US" style="font-family:Consolas">Stanislaw Wojtkiewicz, PhD<u></u><u></u></span></p><p class="MsoNormal"><span lang="EN-US" style="font-family:Consolas"><u></u> <u></u></span></p><p class="MsoNormal"><span lang="EN-US" style="font-family:Consolas">---<u></u><u></u></span></p><p class="MsoNormal"><span lang="EN-US" style="font-family:Consolas">School of Computer Science<u></u><u></u></span></p><p class="MsoNormal"><span style="font-family:Consolas">University of Birmingham<u></u><u></u></span></p><p class="MsoNormal"><span style="font-family:Consolas">B15 2TT<u></u><u></u></span></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal"><u></u> <u></u></p></div><div id="m_-9174478099627014613DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2"><br>
<table style="border-top:1px solid #d3d4de">
<tr>
<td style="width:55px;padding-top:13px"><a href="https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=emailclient" target="_blank"><img src="https://ipmcdn.avast.com/images/icons/icon-envelope-tick-round-orange-animated-no-repeat-v1.gif" alt="" width="46" height="29" style="width:46px;height:29px"></a></td>
<td style="width:470px;padding-top:12px;color:#41424e;font-size:13px;font-family:Arial,Helvetica,sans-serif;line-height:18px">Virus-free. <a href="https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=emailclient" style="color:#4453ea" target="_blank">www.avast.com</a>
</td>
</tr>
</table><a href="#m_-9174478099627014613_DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2" width="1" height="1"> </a></div></div>_______________________________________________<br>
Nirfast mailing list<br>
<a href="mailto:Nirfast@public.kitware.com" target="_blank">Nirfast@public.kitware.com</a><br>
<a href="https://public.kitware.com/mailman/listinfo/nirfast" rel="noreferrer" target="_blank">https://public.kitware.com/mailman/listinfo/nirfast</a><br>
</blockquote></div>