search

NISOx-BDI / SwE-toolbox

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

TFCE bug - find Assertion failed #63

Closed nicholst closed 5 years ago

nicholst commented 5 years ago

Use for issues related to the new implementation of TFCE (PR #48, Issue #15)

nicholst commented 5 years ago

From 'Sid Chopra'

Thanks for sharing the pre-release. I gave it a spin and while the voxel and cluster-level inference still runs fine, I get the below error with TFCE inference: Unable to perform assignment because the left and right sides have a different number of elements.

Error in swe_cp_WB (line 1643)
            scorevol(sub2ind(DIM,XYZ(1,:),XYZ(2,:),XYZ(3,:))) =
            swe_invNcdf(pvol);

MATLAB: management.cpp:671: find: Assertion `' failed.
MATLAB: management.cpp:671: find: Assertion `' failed.

This error seems to occur after the first bootstrap:

Initialising parameters                 :                        ...done
Output images                           :                 ...initialised
...initialised
Plane 121/121, block   1/1              :                        ...done
Saving results                          :                     ...writing
Bootstrap # 1  Block 10/10              :

Please let me know if you require further info.

nicholst commented 5 years ago

@TomMaullin Ideas on this?

TomMaullin commented 5 years ago

Hi @nicholst ,

I believe, after going over the code in a lot of detail, I have found the cause of this bug. I believe it is probably due to the variable blksz not being used correctly. This variable is used for handling the situation when there is not enough memory available. The SwE toolbox breaks the analysis into blocks of size blksz and then computes block by block. The P values are split into their relevant block sizes in the current code (e.g. size(pvol) = [1 blksz]) but the XYZ coordinates are not (e.g. size(XYZ) = [3 {mask size}] ~= [3 blksz]).

The reason I had not come across this error during testing was most likely due to the fact the example analyses I use do not take as much memory and are much simpler than what I imagine many users are doing.

I hope to have a PR for a fix for this shortly. I should note this is only a guess of the cause of this error, as I cannot easily replicate this bug on my computer.

nicholst commented 5 years ago

Thanks @TomMaullin!

sidchop commented 5 years ago

Thanks! TFCE inference now seems to be working smoothing.

TomMaullin commented 5 years ago

Ah great! Thank you Sid!

sidchop commented 5 years ago

Hey Tom, Just wanted to check if there were any substantive differences in the TFCE calculation between the SwE version you sent me (2.0.0rc) and the recently released version. Will I need to re-run my analyses?

Thanks, Sid

TomMaullin commented 5 years ago

Hi Sid,

I believe the TFCE calculation itself is identical to the version you were using! The largest differences between v2.0.0rc and v2.0.0 were that:

In short, the only fixes were for analyses that would not run in v2.0.0rc. If your analysis ran in v2.0.0rc, I believe it should be the same as you would have obtained in v2.0.0.

Cheers, Tom M