add logic to adjust TR to seconds for fslmerge -tr commands

[cmpetty]

added logic for TR in any files that used fslmerge, since it is expecting TR in seconds only

[glasserm]

Hi cmpetty,

Are these changes tested?

Thanks,

Matt.

[cmpetty]

@glasserm

yes to the ones called from GenericfMRIVolumeProcessingPipeline.sh, which should cover OneStepResampling and mcflirt. Will try ICAFix tomorrow, which isn't something we've been running

[mharms]

@cmpetty Thanks for assembling this, but I think it needs a couple tweaks:

1. Did you test this in the context of the standard situation in which the TR_units are already in seconds? I don't see how this could work, as the continue and break constructs are for controlling for and while loops, and using continue/break will thus mess up the intended flow through any surrounding for/while loops, or generate an outright error if the new logic is not embedded in a for/while loop. Please use true rather than continue/break, or depending on our decision on the following point, simply don't check for the ${TR_units} = "s" condition at all.

2. Are "s", "ms", and "us" the full set of allowed TR_units values? Can TR_units sometimes be undefined or empty? In particular, I'm wondering if we should abort the script via our log_Err_Abort function if TR_units is not one of those 3 strings?

3. The value of the TR gets used in hcp_fix_multi_run and ReApplyFixMultiRunPipeline.sh in places beyond just the fslmerge -tr calls, and is expected to be in units of seconds in those other usages as well. Rather than defining a new TR variable in any of the scripts (e.g., TR_vol), let's just use the script's natively defined tr variable, and reassign its value if it isn't in seconds. Thus, any other usages of the TR within the script will be automatically correct.

[mharms]

@coalsont Other than what I just mentioned above, do you see any other necessary changes to this PR, to resolve issue #124?

[coalsont]

Personally, I would use case instead of if/else, and there are unnecessary semicolons at the end of the math lines.

Nifti also defines hertz, ppm, and rads as additional units that could be used instead of time, no idea if fslval supports them. It would be good to at least warn when the fslval output is not recognized as a time unit, but perhaps they should be treated as if they were seconds rather than erroring.