search

NISOx-BDI / SwE-toolbox

SwE toolbox
GNU General Public License v2.0
16 stars 7 forks source link

Add the option to constrain volume clusters to be within the ROI boundaries #149

Closed BryanGuillaume closed 4 years ago

BryanGuillaume commented 4 years ago

This PR is linked to issue #140. It aims to add the option to constrain volume clusters to be within the volume ROI boundaries.

First, this PR refactors the specification of CIFTI additional information as can be observed below:

Screenshot 2019-11-05 at 16 57 52

There is a new option allowing to constrain volume clusters to be within the volume ROI boundaries. Currently, the default value is yes per default to mimic the default behaviour of workbench, but as there seems to be some controversy about what should be the best default behaviour, we may decide to change this if needed.

I have already tested this new feature with a parametric analysis and an example of its effect on the result display can be observed below:

Without ROI Boundary constraint

no_constraint

With ROI Boundary constraint (page 1 & 3)

constraint_2 constraint_3

From these screenshots, we can observe that, when using the constraint, the toolbox is also using a more informative label for the volume brain structure. For example, 'V_AMYL' represents the left amygdala. Here below, the label mapping that the toolbox is currently using (note: 'V' is used as a prefix to those strings in the table):

ciftiVolumeLabels = {... 'CIFTI_STRUCTURE_ACCUMBENS_LEFT', 'ACC_L'; 'CIFTI_STRUCTURE_ACCUMBENS_RIGHT', 'ACC_R'; 'CIFTI_STRUCTURE_ALL_WHITE_MATTER', 'ALL_W_M'; 'CIFTI_STRUCTURE_ALL_GREY_MATTER', 'ALL_G_M'; 'CIFTI_STRUCTURE_AMYGDALA_LEFT', 'AMY_L'; 'CIFTI_STRUCTURE_AMYGDALA_RIGHT', 'AMY_R'; 'CIFTI_STRUCTURE_BRAIN_STEM', 'BR_ST'; 'CIFTI_STRUCTURE_CAUDATE_LEFT', 'CAU_L'; 'CIFTI_STRUCTURE_CAUDATE_RIGHT','CAU_R'; 'CIFTI_STRUCTURE_CEREBELLAR_WHITE_MATTER_LEFT', 'CR_W_M_L'; 'CIFTI_STRUCTURE_CEREBELLAR_WHITE_MATTER_RIGHT', 'CR_W_M_R'; 'CIFTI_STRUCTURE_CEREBELLUM', 'CER'; 'CIFTI_STRUCTURE_CEREBELLUM_LEFT', 'CER_L'; 'CIFTI_STRUCTURE_CEREBELLUM_RIGHT', 'CER_R'; 'CIFTI_STRUCTURE_CEREBRAL_WHITE_MATTER_LEFT', 'CL_W_M_L'; 'CIFTI_STRUCTURE_CEREBRAL_WHITE_MATTER_RIGHT', 'CL_W_M_R'; 'CIFTI_STRUCTURE_CORTEX', 'COR'; 'CIFTI_STRUCTURE_CORTEX_LEFT', 'COR_L'; 'CIFTI_STRUCTURE_CORTEX_RIGHT', 'COR_R'; 'CIFTI_STRUCTURE_DIENCEPHALON_VENTRAL_LEFT', 'D_V_L'; 'CIFTI_STRUCTURE_DIENCEPHALON_VENTRAL_RIGHT', 'D_V_R'; 'CIFTI_STRUCTURE_HIPPOCAMPUS_LEFT', 'HIP_L'; 'CIFTI_STRUCTURE_HIPPOCAMPUS_RIGHT', 'HIP_R'; 'CIFTI_STRUCTURE_OTHER', 'OTH'; 'CIFTI_STRUCTURE_OTHER_GREY_MATTER', 'OTH_G_M'; 'CIFTI_STRUCTURE_OTHER_WHITE_MATTER', 'OTH_W_M'; 'CIFTI_STRUCTURE_PALLIDUM_LEFT', 'PAL_L'; 'CIFTI_STRUCTURE_PALLIDUM_RIGHT', 'PAL_R'; 'CIFTI_STRUCTURE_PUTAMEN_LEFT', 'PUT_L'; 'CIFTI_STRUCTURE_PUTAMEN_RIGHT', 'PUT_R'; 'CIFTI_STRUCTURE_THALAMUS_LEFT', 'THA_L'; 'CIFTI_STRUCTURE_THALAMUS_RIGHT', 'THA_R'};

That is a labelling that I have made up quickly. Thus, we can change that if needed.

Regarding the effect of this boundary constraint, we can, for example, observed that the cluster of 84 voxels observed in the first screenshot seems to be split into 2 clusters, one of 74 voxels in the left amygdala (see page 1) and one of 10 clusters in the left hippocampus (see page 3). This splitting makes sense, indicating that the code is likely to work properly.

I am currently running a WB analysis to check if there are no bugs at this level. Regarding this, it is important to note that the null distribution of volume clusters (for the box-cox transformation) is not done per ROI but for the whole volume. I am wondering if this could be an issue as some ROIs might be smaller than others.

@nicholst, if you have any comment about this PR...

nicholst commented 4 years ago

This is great!

I have some suggestions on how to improve the abbreviations… I will send those later.

nicholst commented 4 years ago

See my PR onto your branch.

BryanGuillaume commented 4 years ago

It took me a bit of time to make this happens, but, now, the display of results for CIFTI outputs is now "clickable" like for a classic volume analysis and the long CIFTI labels are printed when clicking on their abbreviations. Just to summarise the effects of the last commits:

  1. Before, every object in the display figure (the table, volume, title, design matrix,...) could be rotated when clicking on them; now, only the surfaces can be rotated.
  2. Before, even after disabling the rotation of an object, nothing would happen when clicking on it; now, when we click on it, the same event than for a classic volume analysis occurs.
  3. Now, when clicking on a structure label abbreviation, the corresponding long CIFTI labels is also printed as it can be observed below:
Screenshot 2019-11-11 at 14 22 07

I will check if these changes do not create issues with other modalities (GIFTI and NIFTI), but, after that, I believe I can merge this PR.

nicholst commented 4 years ago

Fantastic!

If the fix is discrete, can you please email Guillaume the trick, pointing to our code, so he can add the same fix to SPM?

On Mon, Nov 11, 2019 at 1:45 PM Bryan Guillaume notifications@github.com wrote:

It took me a bit of time to make this happens, but, now, the display of results for CIFTI outputs is now "clickable" like for a classic volume analysis and the long CIFTI labels are printed when clicking on their abbreviations. Just to summarise the effects of the last commits:

  1. Before, every object in the display figure (the table, volume, title, design matrix,...) could be rotated when clicking on them; now, only the surfaces can be rotated.
  2. Before, even after disabling the rotation of an object, nothing would happen when clicking on it; now, when we click on it, the same event than for a classic volume analysis occurs.
  3. Now, when clicking on a structure label abbreviation, the corresponding long CIFTI labels is also printed as it can be observed below:

[image: Screenshot 2019-11-11 at 14 22 07] https://user-images.githubusercontent.com/4426961/68591399-f6c30880-0490-11ea-8c6b-b893dcadbd40.png

I will check if these changes do not create issues with other modalities (GIFTI and NIFTI), but, after that, I believe I can merge this PR.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NISOx-BDI/SwE-toolbox/pull/149?email_source=notifications&email_token=ABHKYQ63YYOF42TTDN26KZ3QTFOZJA5CNFSM4JJFDNCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDW34WQ#issuecomment-552451674, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABHKYQ2566TFD2GO5HDBOD3QTFOZJANCNFSM4JJFDNCA .

--

Thomas Nichols, PhD Professor of Neuroimaging Statistics Nuffield Department of Population Health | University of Oxford Big Data Institute | Li Ka Shing Centre for Health Information and Discovery Old Road Campus | Headington | Oxford | OX3 7LF | United Kingdom T: +44 1865 743590 | E: thomas.nichols@bdi.ox.ac.uk W: http://nisox.org | http://www.bdi.ox.ac.uk

BryanGuillaume commented 4 years ago

I have caught and fixed a bug where the rotate mouse pointer would appear above the contrast matrix in the result display. I have also added some try statement to prevent issues with Octave which has limited functionalities with rotate3D.

From testing, I could not find any other issues. Thus, I am merging this PR.