Just to confirm whether I understand it correctly:

• fod2fixel is unaffected; i.e. the -peaks option was fixed with fix SH precomputer #1206; has been correct since; is not affected by this PR. Also fod2fixel -afd was never affected, not before nor after fix SH precomputer #1206, so also not by this PR.
• sh2peaks was never affected, not before nor after fix SH precomputer #1206, so also not by this PR. However, might want to double-check what's exactly the case for the sh2peaks -fast option.
• tckgen is affected; i.e. was in fact correct before fix SH precomputer #1206 (?); has been "broken" since; so is affected by this PR. This affects the amplitudes as evaluated during tckgen, so basically as well whether a given amplitude is above/below -cutoff.

So in conclusion of all the above, my understanding is that this PR only affects tckgen, correct?

If tckgen was fine before #1206, and given that the "fix" back then was orientationally dependent (i.e. on the inclination angle of spherical polar coordinates); this means this PR also causes an orientationally dependent change (again), right?

This might very well catapult the sensible -cutoff value or range back again too; but wrt that one, I think it's at this point more clever to first fix this, wrap up the dwi2response dhollander business and take into account mtnormalise. And maybe consider first whether there's anything else that's going to affect the actual amplitudes -cutoff is typically applied to. And then evaluate what's a sensible value for -cutoff.

Great catch @jdtournier !

OK, just to clarify things (although it might complicate matter further):

• fod2fixel -peaks was affected before fix SH precomputer #1206, fixed by fix SH precomputer #1206, and remains unaffected here.
• fod2fixel -afd remains unaffected as far as I can tell (doesn't rely on SH precomputer)
• sh2peaks was unaffected before fix SH precomputer #1206, the -fast option was added in fix SH precomputer #1206 to allow use (and testing) of the SH precomputer, and remains unaffected here (checks out in testing).
• tckgen with iFOD1 or iFOD2 was unaffected before fix SH precomputer #1206, messed up after fix SH precomputer #1206 with this bug, and fixed with this PR.
• tckgen with SD_STREAM must have been affected before fix SH precomputer #1206 (although I've not checked that), since it uses the SH precomputer to get the peaks - the exact piece of code that was problematic with fod2fixel -peaks. So it must have been fixed with fix SH precomputer #1206, and should still be fine now.

So it's a bit of a messy story...

On a different note, I've double-checked that tckgen now produces the correct output, it that the output is strictly identical with and without -noprecompute (the default is to use the SH precomputer, which is what triggered the bug). I'll add further tests for that shortly.

Thanks for taking the time to clarify; it's appreciated and useful.

fod2fixel -peaks was affected before #1206, fixed by #1206, and remains unaffected here.
fod2fixel -afd remains unaffected as far as I can tell (doesn't rely on SH precomputer)
sh2peaks was unaffected before #1206, the -fast option was added in #1206 to allow use (and testing) of the SH precomputer, and remains unaffected here (checks out in testing).

Ok, good to see that clarified. Makes sense; so those are all fine and unaffected.

tckgen with iFOD1 or iFOD2 was unaffected before #1206, messed up after #1206 with this bug, and fixed with this PR.

tckgen with SD_STREAM must have been affected before #1206 (although I've not checked that), since it uses the SH precomputer to get the peaks - the exact piece of code that was problematic with fod2fixel -peaks. So it must have been fixed with #1206, and should still be fine now.

Hmm, ok, indeed messier than I anticipated; at least it's contained to tckgen only, even to the probabilistic algorithms only.

Other than the dwi2response discussion though, here mtnormalise can't play the role of equaliser, since this change's effect happens after any such steps. Once all of these, and possibly the angle constraint for SD_STREAM too, wrap up, it'll definitely be a good idea to re-evaluate -cutoff defaults again.