Bug fix to makeSubjectTaskSummary

[mharms]

I'm not entirely clear on the purpose of makeSubjectTaskSummary yet, esp. in the context of subjects with multiple runs for a given task and thus a "Level2" analysis, but it seems clear to me that it contained a bug in its treatment of $SummaryName in the situation of a Level2 analysis. Specifically $SummaryName would be of the form "A/A" which will then create problems when creating files with $SummaryName as part of the file name.

I've attempted to resolve this by splitting --summaryname into separate --summarydir and --summaryprefix arguments.

This hasn't been tested. I was hoping that @glasserm could conveniently test it.

[coalsont]

My thought on the recent error is that someone misunderstood the description of that argument (which contains a slash that I think is intended to indicate "or", considering the logic of whether the thing gets run or not), and that it wasn't intended to support an extra subdirectory. Putting a / into, say, --subject would also cause weird "subdirectory" problems.

[mharms]

No, it's more than that. If you look at Greg's description of --summaryname, you see that he in fact referred to it as containing both a directory and file prefix component:

opts_AddOptional '--summaryname' 'SummaryName' 'tfMRI_TaskName/DesignName_TaskName' "Naming convention for single-subject summary directory. Will not create summary directory for Level1 analysis if this flag is missing or set to NONE. Naming for Level1 summary directories should match naming of Level2 summary directories. Default when running Level2 analysis is derived from --lvl2task and --lvl2fsf options "tfMRI_TaskName/DesignName_TaskName"" 'NONE'

That interpretation is supported by the fact that the code itself in one condition was setting (previous L208):

SummaryName="${LevelTwofMRIName}/${LevelTwofsfName}";

Basically, I think at some point that @gcburgess got a bit twisted up in terms of whether --summaryname was just a prefix, or a dir/prefix combination.

[coalsont]

tabs/spaces mismatch

[coalsont]

Also, if someone doesn't read the help, or just forgets --summaryprefix when using --summarydir, it looks like at present it will use a literal NONE along with the provided --summarydir, and makeSubjectTaskSummary.sh appears to use it as-is in that case.

[glasserm]

Wouldn't it be easier to just fix the description? The current code works fine if you use it more sensibly. I don't see why we need these additional variables.

[mharms]

It doesn't work sensibly in the case of applying it to Level2 outputs. I've emailed @gcburgess for clarification as to what the role of the script is on Level2 data.

[coalsont]

This case would get overwritten by the below logic anyway, and the converse isn't tested for.

[glasserm]

I see it as an either or option. Either summary directory or level 2 directory. We don't need to make this more complicated than that.

[coalsont]

These options sound like they are intended to provide what would have been the level 2 information, so that the directory structure will match what doing the level 2 mode would have done. Should the script instead have a separate yes/no argument to toggle between the two modes of running, and make the level 2 information mandatory (even when it doesn't do the "real" level 2 stuff), instead of the current separate arguments to give the same level 2 information in the non-level-2 scenario?

[glasserm]

That might be an alternative approach. Not sure I see how that simplifies things or clarifies them though.

[coalsont]

If it really is as simple as "the same information, used a different way", it could be less confusing to explain to users than "here are different flags that must be used to provide the same information, but which also change what the script actually does". I'm not clear on the details of when it is applicable without doing the level 2 stuff (only when it was already done previously, such that the flag could be something like --skip-level2 or --use-prior-level2?), and that could affect whether it is actually simpler to explain.

[mharms]

What is the purpose of the parts of the script that are checking if the data is from Level2? If I understand that, I might better understand what Greg was doing ... For example, to the extent that we even need to do anything with Level2 data, why isn't it a simple one-liner where we just link to the Level2 feat directory itself? I feel like I must be missing something...

[mharms]

@glasserm : Have you actually run this script on any Level2 data, so that we have any actual example of what it is generating?

[gcburgess]

The point of makeSubjectTaskSummary.sh was to give a single, smaller, simplified output directory with the same naming convention whether the data came from one scan run or two scan runs.

For HCP-YA data, we had two scan runs for every task. However, when a subject only had one good task run, we excluded their data from public release entirely. One reason for that decision was the different naming conventions for outputs from Level1 for a single run versus Level2 directories from multiple runs.

For Lifespan, most of the tasks have only one run, but some do have two runs. Consequently, the output naming convention can be an issue when you’re trying to understand how to work across two different tasks (e.g., HCD CARIT and EMOTION).

The last reason for including the summary directories was that they don’t include the original or residual time series data, making them a much smaller package option if people want to save disk space.

Regarding the original concern about the $SummaryName option having a slash character ("A/A"), it was intended to capture the Level2 directory naming convention when you don't run a Level2 analyses (don't specify the multiple Level2 options). When that $SummaryName variable contains a slash, it bypasses the need for two arguments (one for parent directory and one for subdirectory). But, yes, it could have been done by requiring two separate arguments for the summary directories.

[glasserm]

I don't understand any of this. Somehow I was able to produce comparable layouts for level2 or summary directory without all this complication.

[gcburgess]

@glasserm Your Level1 outputs will not have the same filenames as your Level2 outputs. That's the purpose of the summary directory: to run an analysis that includes both Level1 and Level2 outputs.

[glasserm]

So I used what I would have used as the argument for level 2 for the summary directory and everything worked without multiple new options or A/A specification.

[gcburgess]

That's correct. If you passed all of the Level2 options into the script, it already knows how to name the directory. So, it doesn't need you to pass in the summary directory naming info.

The issue comes when you run only the Level1 analysis for a subject. At that point, it doesn't know how to name the summary directory to be consistent with the Level2 naming convention, because those Level2 options were not provided. So, you need to pass in options for naming the summary directory when you run only Level1 analysis, to ensure the directory name will be the same as the directory created automatically after running Level2 for the other subjects.

[glasserm]

Right if I were to run level2 I would call it tfMRI_${TASK} (with no phase encoding direction). And for summary directory I would similarly call it tfMRI_${TASK}. When I did this everything worked as I expected it to and I was surprised that there was all this confusion.

[mharms]

@gcburgess : I still don't understand why the MakeSummary script is set up to run on any Level2 data at all ...?

[mharms]

And to clarify, the existing script (prior to the PR) works ok on Level1 data if and only if summaryname is a single string with no slash -- the script then creates a ${ResultsFolder}/${SummaryName} directory, and also uses ${SummaryName} as a component of various file names.

The existing script won't work as is when applied to Level2 data. But per above, I'm confused why we are ever running the script on Level2 data in the first place.

[glasserm]

I don't understand that either. I thought this was a substitute for level2 and that's how it appeared to work when I used it that way.

[mharms]

@gcburgess : If you have a moment to help clarify for us the intended role of the MakeSummary script on Level2 data, that would be great. @glasserm and myself are both confused on why the option for running MakeSummary on Level2 data exists, since at least superficially such a mode of operation doesn't seem necessary if the main purpose of the script is to make Level1 data have a similar naming convention and directory structure to Level2 data.