search

NISOx-BDI / SwE-toolbox

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

SwE files written - Final tidying #81

Closed nicholst closed 5 years ago

nicholst commented 5 years ago

Hi @TomMaullin,

In going through the files written with a fine-toothed comb I've come across a couple of quirks that I wanted to flag. You can tell me how many of these are 'features' vs. unintended actions. Some of this is really about the man.html documentation but I'll include it here for clarity (see https://github.com/NISOx-BDI/NISOx-BDI.github.io/pull/67)

nicholst commented 5 years ago

Noting location of spreadsheet with all this info: https://docs.google.com/spreadsheets/d/1uDNkcQ8GE_u5e7zEruGScaxPFciwrdbG-X895XM-8PU

See both tabs.

TomMaullin commented 5 years ago

Hi @nicholst ,

There's quite a lot going on here - I'll respond one point at a time:

Is edf always written out for .mat file input? I can't tell, but it also looks like mat file output never got fully updated with the new filenaming. See https://github.com/NISOx-BDI/SwE-toolbox/blob/master/swe_cp_WB.m#L1199-L1241

For .mat input, I believe the only case an edf map isn't output is classic parametric inference (where dof_type=0 - see here). In the wild bootstrap it is always output (see here). As far as I'm aware .mat output was updated to match the new format - have you found a file that is incorrectly named?

Parameter estimates "beta" are named strange. According to man.html, we have swe_vox_beta_c{c#}.mat and swe_vox_beta_c{c#}.nii but beta's are independent of contrast number, right? And so shouldn't have a "_c"... right? Did we ever write out contrast estimates? These are con_XXXX.img in SPM or copeXXXX.nii.gz in FSL.

I'll respond to these two at the same time as I'm fairly sure that swe_vox_beta_c{c#}.nii is con_{c#}.nii... this file doesn't seem to be written out in every scenario but I am fairly certain it is written out in all parametric scenarios (see here for .nii and here for .mat). It is a little hard for me to verify this output for certain as I have only touched these parts of the code when renaming files and many of the original variables around here (and the analogous sections for WB) are named things like tmp and tmp2. But I can confirm that before renaming, swe_vox_beta_c{c#}.nii was called con_{c#}.nii (I believe this change of filename came from your instructions here for renaming).

The bb in the swe_vox_beta_bb.mat name doesn't follow... we've used vv to indicate a nVisit x nVisit matrix is being saved, but there's not 'nBeta x nBeta' thing being saved; this is just a beta-vector. For matrix input this can just be swe_vox_beta.mat. (The single b in swe_vox_beta_b{b#}.nii does make sense though).

Ah yes apologies I'll change this!

TomMaullin commented 5 years ago

Noting location of spreadsheet with all this info: https://docs.google.com/spreadsheets/d/1uDNkcQ8GE_u5e7zEruGScaxPFciwrdbG-X895XM-8PU

See both tabs.

I have looked through this table and agree with what is written... I have changed one or two entries in the first table to hopefully add clarity - if this seems unclear for you please feel free to revert my changes! I will look into changing the website documentation once the TFCE PR is merged.

nicholst commented 5 years ago

Responding to each item...

TomMaullin commented 5 years ago

This issue has now been addressed by PR #48