Random Improvements

[nicholst]

The following changes will make for easier usage and encourage good practice

• The naming of the input variables is inconsistent: Y_files and X suggests that X is not a file but the actual design matrix. Use some sort of consistent naming... maybe Y_files and X_file?
• The design matrix should, to encourage good practice, show require or at least allow column headers and row ID's. Either we just demand it, or expect a given behaviour based on extension. E.g. if "csv" or "tsv", then file must have row header and "column header" (i.e. first row and first column are ignored), while if "dat" then row or column headers are expected and nothing is ignored. The same pattern could be extended to Y and M files... if "csv" or "tsv" then first row and column are ignored, if "txt" (or, well, anything else) a single column is expected.
• Filenames should be able to be specified without extension... so you're not dependent on whether the images happen to be .nii, .nii.gz or .{hdr,img}. Might need nibabel's filename_parser.
• To avoid an explosion of files, consider saving blm_vox_cov* files as 4D file. Also, any file that you number, use at least 2 digits padded by zeros.
• In addition to Tstat there should be a corresponding con and se image, for the numerator and denominator of the Tstat. For Fstat we could also write out a R^2 image.
• Revise contrasts specification to remove "statType"; that should be inferred from the use of a double list, i.e. [1,0,1] vs [[1,0,1]].

[TomMaullin]

Hi Tom,

Ah okay, in response to a few of these points;

* The naming of the input variables is inconsistent: `Y_files` and `X` suggests that X is not a file but the _actual_ design matrix.  Use some sort of consistent naming... maybe `Y_files` and `X_file`?

• The reason I chose this naming was that Y_files is a list of files contained in a file, whereas X is just a file containing the design matrix. By the logic above the consistent naming I guess would be Y_files_file and X_file as the file for Y contains a list of Y_files and the file for X just contains the design matrix X.

* In addition to Tstat there should be a corresponding `con` and `se` image, for the numerator and denominator of the Tstat.  For Fstat we could also write out a R^2 image.

• For the T stats, would this be a map of c\beta and var(c\beta)? If so these are already output as blm_vox_beta_c{#c} and blm_vox_cov_c{#c} following the naming convention we used in SwE. Similarly an R^2 is already output for F contrasts as blm_vox_pr2_c{#c}.

All other points make sense to me and would not take too long to implement.

[nicholst]

OK... I see your logic on Y_files and X.

I don't see any blm_vox_beta_c{#c} or blm_vox_cov_c{#c}... I see the likes of blm_vox_beta_b1.nii and blm_vox_cov_b1,1.nii... i.e. for individual betas but not the contrasts.

For contrasts I think the full set of results that makes sense is

• blm_vox_con_c{#c}
• blm_vox_se_c{#c} or blm_vox_var_c{#c}
• blm_vox_Tstat_c{#c}
• blm_vox_Tstat_lp_c{#c}

and then

• blm_vox_r2_c{#c}
• blm_vox_Fstat_c{#c}
• blm_vox_Fstat_lp_c{#c}

Again, not super high priority, but do you agree this makes sense?

[TomMaullin]

Ah apologies... the blm_vox_beta_c{#c} and blm_vox_cov_c{#c} files should have been output... were there any errors in the logs when you ran your analysis? Please could you paste the contrast section of your blm_config.yml file here?

But yes, all files apart from those are named as you have said; I will change these last two in my next PR!

[nicholst]

Doh! You're right! I lost track of it among all the other beta's and covs.

For future reference, this is where I'm playing with the BLM code: /users/nichols/kfh142/group/projects/UKB/ANALYSIS/BLMtesting

I think the naming could be better though... "beta" to refer to a contrast is confusing, and as a contrast will never be anything but a scalar, calling it's variance "cov" also seems confusing.

I'm going to re-create the 'random improvements' list in a new message below.

[nicholst]

The following changes will make for easier usage and encourage good practice

• [ ] The design matrix should, to encourage good practice, require column headers and row ID's when used with "csv" or "tsv" (i.e. first row and first column are ignored); any other suffix will be assumed to be white-space separated text (i.e. compatible with Matlab, if that's what people want).
• [ ] The Y, X and M files... should also require column headers and row ID's when used with "csv" or "tsv". Again, if any other suffix is used then it is just plain white-space separated text (or for Y and M, just one filename per line).
• [ ] To avoid an explosion of files, consider saving everything that can be as a 4D file. The prime example is blm_vox_cov_bX,X* which will explode with P, but beta and con and even Tstat images can benefit from this too. (Will need to provide very clear documentation on how cov_bX,X is unwraped.)
• [ ] If not using a 4D file, pad filename's digits with zeros to use 2 or 3 places (this is a non-issue if using 4D file).
• [ ] Filenames should be able to be specified without extension... so you're not dependent on whether the images happen to be .nii, .nii.gz or .{hdr,img}. Might need nibabel's filename_parser.
• [ ] Revise contrasts specification to remove "statType"; that should be inferred from the use of a double list, i.e. [1,0,1] vs [[1,0,1]].
• [ ] Naming of T contrast outputs. Currently we have blm_vox_{beta,cov,Tstat,Tstat_lp}_c1.nii. I believe it will be less confusing if it is: blm_vox_{con,conSE,conT,conTlp}_c1.nii for contrast estimate, contrast standard error, contrast T-value and contrast T's -log p; this makes it clear that these are all derived from a contrast. (Note the removal of the additional underscore (conTlp instead of conT_lp for consistency, and also that cX will go if all-4D-output is used).
• [ ] Naming of F contrast outputs. Currently I think we have blm_vox_{Fstat,Fstat_lp}_c1.nii. Here I think we should have: blm_vox_{conR2,conF,conFlp}_c1.nii for contrast partial R², contrast F statistic and contrast F's -log p.

[TomMaullin]

This issue has been seperated into #69 , #70 , #71 and #72 respectively.