Seems there isn't actually a fixel2peaks command; probably doesn't make sense to add one without the other...

I'm a bit confused - isn't that already possible using fixel2voxel split_dir...? If so, there is an argument for saying that fixel2peaks is easier to find, but I would personally prefer to keep all fixel-space to voxel-space conversions in those commands. So on that basis I'd argue your new peaks2fixel should be subsumed within voxel2fixel - I think all that would be needed would be to accept 4D inputs...?

isn't that already possible using fixel2voxel split_dir...?

Hmm, forgot about that. Maybe that's not where it should be...

Partly true: you can get a peaks image, the peak amplitudes aren't actually currently influenced by the input fixel data file though, which they should be. The other issue is that you can't trivially generate a unit-norm peaks image using fixel2voxel split_dir since the input must be a fixel data file; whereas fixel2peaks could do that fairly sensibly by instead providing the directory / index or directions image as input. split_dir is very much an outsider in that command: even for split_data the nature of the output data is reasonably similar to the other operations, but split_data isn't actually producing what we would classify as "voxel" anywhere else.

So on that basis I'd argue your new peaks2fixel should be subsumed within voxel2fixel - I think all that would be needed would be to accept 4D inputs...?

That one's harder to sell: the existing functionality is limited purely to "voxel" images, this functionality would work exclusively for 3-vector images, and there's already another "peaks2*" command; so it'd be expecting a lot of users to look in that location. Our command naming convention only works if it's actually stuck to.

That change is what makes sense to me.

The fixel directory format and 4D 3-vector "peaks" images are so intimately linked - it's the same nature of data just stored differently, the mrview fixel plot tool opens either transparently - that I think it makes sense to have appropriately named commands that convert between the two. fixel2voxel split_dir is actually a kind of obscure way of achieving one of those conversions, and it also isn't currently feature-complete (it only outputs unit-norm vectors, not that that wouldn't be easy to fix).

It also means looking at fixel2voxel split_data differently. That naming was because split_data and split_dir needed to be distinguished. But if you think about it, there's actually no "split" operation going on: the data within each voxel are remaining split, i.e. not being combined using some statistic. So you can think of that fixel-to-voxel conversion not actually applying any mathematical operation in the course of said conversion. Hence I've renamed it to "none".

I know there'll be pushback on incrementing the command cardinality... but I maintain that sensible command naming and interfaces & feature discoverability take precedence over such. Hell, I wrote code for a new command because I myself forgot fixel2voxel split_dir was there, and I'm a co-author on that command.

# command: fixel2peaks fixel_image/ - | testing_diff_peaks - fixel2peaks/nodata.mif 1e-6
 [ ERROR ]
fixel2peaks: [ERROR] Could not locate fixel data based on input string
fixel2peaks: [ERROR] Error when interpreting as image: 
fixel2peaks: [ERROR]   unknown format for image "fixel_image/"
fixel2peaks: [ERROR]   error opening image "fixel_image/"
fixel2peaks: [ERROR] Error when interpreting as fixel directory: 
fixel2peaks: [ERROR]   Input path is not a directory
testing_diff_peaks: [ERROR] no filename supplied to standard input (broken pipe?)
testing_diff_peaks: [ERROR] error opening image "-"

Only occurs in Windows CI build.

Our command naming convention only works if it's actually stuck to.

I couldn't agree more, but the word "peak" refers to a maximum, as in sh2peaks, which indeed computes a maximum. peaks2amp can indeed work "more generally", but it does still apply to peaks as well; just like there's other commands that have more creative uses. This reveals an interesting difference in how specific or broad the bits before "2" can be, versus those after "2". Those after "2" should probably be more strict, as those names make promises on what's delivered, i.e. postconditions. Those before "2" are preconditions, guaranteeing normal or intentional operation if the user provides as such, but they're not per se strict: if the user provides something else they just take responsibility themselves for whatever happens. So sh2peaks promises peaks, i.e. maxima, and delivers that. peaks2amp suggests to provide maxima (represented as vectors with a magnitude), and delivers amplitudes. However, users will find that it also happens to work if those vectors weren't "maxima" of something; they unlock a valid "creative" (so as to say) use of the command and use it to their benefit. So again, all checks out.

Apply the above to the proposed peaks2fixel and fixel2peaks pair, and you'll find the communication to the user fails for fixel2peaks. To be honest, I think you risk a critical miscommunication to the user here, by mixing the "peaks" naming in with fixel orientations. Fixels can have orientations that can indeed come from various sources, but arguably the currently most common is not the peak of FOD lobes. Indeed, another occurrence of peak is in the relevant option to fod2fixel, which again strongly associates it with a maximum specifically. Putting some of this together, you can imagine the most straightforward of mistakes a user can make here, using the default behaviours of commands, and guided by what their names communicate on a high level of abstraction: fod2fixel ..... | fixel2peaks ...... Obviously (to us), this won't result in peaks of the FOD; but the names tell (or at least suggest) a different story. Yes, surely you can find other examples in the software of similar cases, at least on an abstract level, however, given that the notion of a fixel still is very confusing to the majority of people out there, I don't think you want to take any risks with that one in particular.

To be fair, I don't see what the issue was or is with having this functionality nicely grouped together in fixel2voxel and voxel2fixel; I find that rather handy personally. I have worked with, and am working with, users that have used the split_dirs and even split_data functionalities, and they have never voiced any confusion about it. On the contrary, split_dirs doesn't break their understanding of a fixel when they have (finally) come to see that a fixel isn't a peak (per se).

You need to test these kinds of "...easier to find..." claims with actual users, rather than try to theoretically approach this, or you risk overlooking relevant context (this generally goes for any claims about user behaviour). The context here is that there's only so many fixel2... commands actually present. Go and have a look; you'll find it's hard to miss the correct command if you wanted to turn your fixels into an image that has the directions. In the actual absence of a fixel2peaks command, it's hard to image what other "future" fixel2... command could be more suggestive of offering this functionality (but while actually doing something else), over fixel2voxel that is.

So well, long-winded explanation as always, but the gist is that I would carefully suggest to not mix the "peaks" terminology or wording in these scenarios. I haven't seen issues with split_dirs so far, and I can't imagine why that would suddenly be(come) the case. I keep coming back to:

but I would personally prefer to keep all fixel-space to voxel-space conversions in those commands. So on that basis I'd argue your new peaks2fixel should be subsumed within voxel2fixel - I think all that would be needed would be to accept 4D inputs...?

...and find that very sensible.

All nouns within command names refer not to the quantitative nature of the data contained (with the exception of mr being not quite as general as it could theoretically be), but the requisite knowledge of the format and nature of the data to enable proper interpretation. "voxel", "fixel", "label", "mesh", "dec"; all describe a format, not a specific quantity. "peaks" was utilised to refer to 3-vector images due to sh2peaks being the only command generating such at the time. Sure, it's not perfect, because of the ambiguity in "sh2peaks" between algorithm applied / quantity extracted / format of the output data. But I really don't think that users accepting "peaks" as referring to 3-vector data is more of a stretch than conversion from fixel directory format to 3-vector data being found in "fixel2voxel", especially when such data would be wholly inappropriate to provide as input to either of the two commands currently named "voxel2*".

I'm not sure that "peaks" is entirely problematic anyway. A 3-vector image can be thought of as a function on the sphere in each voxel, that is zero everywhere except for a small set of discrete orientations. In that case, the "peaks" do indeed refer to the maxima of the function in both position and magnitude. fod2fixel's -peak would be better named -max, since it writes the maximum value of the FOD within the fixel and not the "peak" itself (which I do think of as defining not just the function value at the location of a maxima but the position of that maxima within the domain of the function as well); as you say, the fixel direction doesn't even correspond to the peak direction either, so it's doubly problematic. I would genuinely advocate for that option being changed, and some of the other text around the FMLS segmenter being revised accordingly.

If we were going to forbid "peaks" from being used to refer to things that are not function maxima, then peaks2amp would have to change. You couldn't reasonably pipe fixel2voxel into peaks2amp on one hand, be draconian about enforcing one specific interpretation of the word "peak" on the other, and keep a straight face. If "peaks" was instead "3vec" or "xyz" (which might have been proposed at some point) there wouldn't be an issue: this is a conversion of data from one format into another. I grant that "intended for this data, but there's a happy side usage" and "such quantities are just one of a wide range to which this command is applicable" exist on either end of a spectrum, but I'm not convinced the cases under consideration are so disparate so as to justify entirely opposing treatment.

If "peaks" were to be reserved for "sh2peaks" to refer moreso to the algorithm / quantity reflected in the output data rather than its format, and therefore something other than "peaks" were required to refer to 3-vector data, and alternative noun were not to be added, then fixel is actually more appropriate than voxel. The nature of such orientation-based information necessitates special handling in many instances no different from that of the fixel directory format; the data are just stored in a different format. Going from fixel directory format to 3-vector images would therefore be fixelconvert. Some fixel commands might only work with one format and not the other, but it'd be more faithful than voxel with respect to how the data need to be interpreted. Not advocating for this, just highlighting why I think "fixel2voxel split_dir" was a mistake from the beginning.

I would also advise refraining from telling me what to do.

Ok, I agree that there's no point being too prescriptive with our interpretation of what constitutes a ’peak'. As Rob says, the format is probably more important in practice than what the data actually represents. Personally, I think the main thing is for us to express what these commands generally expect to operate on, with the understanding that anything that 'looks like' a peaks image (in this instance) is acceptable.

Regarding the specific issue here: I don't mind either way to be honest. I agree with Rob that there's a good argument for fixel2voxel to literally convert between formats without any further interpretation (other than the genetic operations provided), so it makes sense to replace split_data with none and ditch split_dir in favour of a dedicated command - especially since there's different ways that the peak might be modulated. While this could still easily be performed with fixel2voxel with appropriate, additional options, it's true that there's probably enough special handling there to warrant a dedicated command.

So to cut a long story short, I'm happy enough with these new commands, and removing split_dir. 👍

@jdtournier Probably want to have a look at a26f89d. I confirmed that Path::Dir is not affected.

@jdtournier Probably want to have a look at a26f89d. I confirmed that Path::Dir is not affected.

Looks good to me 👍