Pairwise bootstrap ISC doesn't work

[yibeichan]

Hi,

I'm using bootstrap_isc with pairwise=True. The first time I ran it, I got the error from assert np.allclose(voxel_iscs, voxel_iscs.T) because my isc_data has nan. When we do a comparison, nan is never equal to nan. So I applied np.nan_to_num(isc_data) and I ran a whole night for a (378, 161032) ISC data, neither any results nor errors. It's weird.

My questions are:

1. is assert np.allclose(voxel_iscs, voxel_iscs.T) that necessary to be here? Since nan != nan, whenever we compare two matrices with nans, even if they are identical, we'll always get False.
2. what happened to my second try with np.nan_to_num(isc_data)? There is no error so I don't what was going on. I guess, it just needs more time to do the computation.

Thank you!

[CameronTEllis]

Hello! From my understanding, you diagnosed the problem correctly: assert np.allclose(voxel_iscs, voxel_iscs.T) won't work if there are nans, so using np.nan_to_num 'fixed' that issue; however, it ran overnight and didn't finish because it is slow to run 1000 bootstraps. You could test if that is the problem by setting n_bootstraps to a small number (e.g. 5) and seeing if that finishes.

More generally, I would be cautions about using np.nan_to_num. If you have nans, how do you want to deal with those? Are all voxels for a given TR nans or are some voxels all nans or is it something between?

[snastase]

Hi @yibeichan, thanks for pointing this out! Like @CameronTEllis said, you're correct that it breaks when receiving nans specifically when pairwise=True because of the assert np.allclose(voxel_iscs, voxel_iscs.T) line. This is a bug. The original thought behind that assertion was to ensure that the user correctly inputs pairwise ISCs in distance vector format so that squareform properly yields a square symmetric matrix, but I didn't foresee the issue with nans.

Thinking about how best to deal with this... Looking closer at the code, I think that the preceding _check_isc_input line already takes care of checking that pairwise ISC inputs are correctly formatted as a distance vector, which makes the assertion(s) in question superfluous. So I think they can be dropped completely. We can add force='tomatrix' to the squareform call, which I think will ensure the expected behavior, if we want to be extra safe. @CameronTEllis does this make sense to you?

[CameronTEllis]

@snastase I agree with your suggested fix, it seems like that assertion is unnecessary given the other checks.

[yibeichan]

Thank you all! @CameronTEllis yes, I have too many voxels. I tried it on a small set. It worked. What do you mean Are all voxels for a given TR nans or are some voxels all nans or is it something between? Since my input is an ISC matrix with the shape as (num of pairs, num of voxels), I don't have any TR information anymore. So what's your concern?

@snastase do we really need to add force='tomatrix' to the squareform call in _check_isc_input ? Doesn't assert voxel_iscs.shape[0] == voxel_iscs.shape[1] in bootstrap_isc do the same thing?

[yibeichan]

oh yeah, assert voxel_iscs.shape[0] == voxel_iscs.shape[1] only checks the shape, but force='tomatrix' actually returns you a matrix. But I thought a matrix is default if the input is in the correct shape?

I'm just curious when we will get a vector, not a matrix from squareform() if we don't specify force='tovector'. I mean, do we have to add force='tomatrix' in _check_isc_input?

[snastase]

@yibeichan squareform is kind of like a toggle: if you input a distance vector, it outputs the distance matrix; if you input a distance matrix, it outputs the distance vector. Those assert statements probably snuck in there when I was initially writing the code (and hadn't thought about force='tomatrix'). I don't think the force='tomatrix' is strictly necessary given the tests in _check_isc_input, but I don't mind a little redundancy if it makes the squareform easier to interpret.