Switch to Unitful.jl

[ssfrr]

(from #29) SIUnits.jl has been unmaintained for a while and Unitful is the future. Should be pretty easy to switch.

In general I've struggled a bit to figure out where units are really useful. At first I made SampledSignals pretty unit-full, e.g. samplerate(buf) would return the samplerate in Hz. The issue that came up was that often when I was doing other computations they weren't unitful, so I was constantly needing to strip and add units as I got things in and out of SampledSignals. Where I've come to now is using units as a convenience UI, so you can do things like indexing a buffer in seconds (or Hz for frequency-domain buffers), but generally the API functions return unitless floats, and you don't need to supply unitful arguments in general. I'd be curious to hear your experience for where the units have worked well. (amplify is definitely a case where it's great to be able to use both amplify(0.5) or amplify(-6dB), though it might be nice to just add a * method).

from @haberdashPI

The main advantage I have found in using units is the readability of the API. E.g. tone(200Hz,500ms) is a lot clearer than tone(200,500). Sometimes I want to compute a number of samples, and there, units are also pretty handy, e.g. floor(Int,50ms * samplerate(sound)). This seems safer and not much more verbose than the unitless version. These are my most common use cases so I decided to return a unitful value from samplerate.

An older version of the API assumed a particular unit and automatically added the units and threw a warning. E.g. tone(200,500) would throw a warning indicating that it was assuming a 200 second long tone at 500 Hz was the inteded output. However, I never found myself needing to use the functions without units, and it's really easy to add units, so it seems simple enough to just make them errors instead of warnings. It would be easy to change them back to warnings (it just requires changing the definitions of my inHz, inseconds and inframes methods).

For more general use, maybe it makes sense to have a unitless and unitful version of samplerate (samplerate and samplerate_Hz??), or maybe it's easy enough to just write samplerate(x)*Hz when you need the units. Not sure about that.

There is one place where I found units to be annoying, and it is the only place in my current API where I don't employ units: defining a sound in terms of a function, like so:

sound = Sound(t -> 1000t .% 1,2s) |> normpower

A unitful version of this would look something like the following

sound = Sound(t -> uconvert(unit(1),1kHz * t) .% 1,2s) |> normpower

Which I find a lot less readable, and there is little value added.

However, I really think this is a problem with the implementation of Unitful. It seems like in this particular case, Unitful should be able to know that the resulting number is unitless, and should convert the result to an appropriate Real type. Maybe that's an issue I should submit or even make a pull request for...

Regarding amplify, I also implemented * and / on Real and Sound inputs, but in a chain of operations I find |> amplify(10dB) more readable than |> x -> x*10dB. In a one off manipulation you can still do x = 20dB * x. Perhaps I could eschew amplify if JuliaLang/julia#24990 gets merged, since |> 20dB * _
is fairly readable.

followup from @haberdashPI

Bottom line - the absence of any further impetus, I say the rule should be, unitful inputs, unitless outputs.

I think there's argreement that units are useful as input to functions. But it sounds like unitful outputs can be problematic in your experience.

One argument against avoiding units is that internal calls, not the public the API, can avoid unitful values in general. In Sounds I regularize all inputs using inHz, inseconds and insamples as appropriate, and then all internal functions have well defined interfaces in terms of these three quantities. Thus,
the internals of Sounds doesn't have to worry about stripping units. For instance, right now I have a unitless version of samplerate, which I somewhat confusingly call rtype (since it is the rate type parameter, typically denoted R of the Sound object).

However, I'm currently, slightly inclined to take the conserative approach and change my design in Sounds to use a unitless samplerate to avoid the problems you note. The rule would be to use unitful values for input and unitless values for output. It would be a very consistent approach to
document. It's also not much longer to type samplerate(x)*Hz than to type
samplerate(x), when necessary.

The one somewhat akward thing about this approach is that there might be cases where a user wants to pass output, from samplerate for instance, as input to another function in the API, and it seems strange that they'd have to specify the units manually, since we already know what the units are. What I'd really like to be true is for Unitful to know how to strip units away when they cancel out. I've submitted an issue for that here: JuliaPhysics/Unitful.jl#128.

However, that's a hypothetical set of use cases, and the only real use case I can think of where units are more convienient in the output, is for using samplerate to compute some sample length from a duration (also specified as a unitful quantity), for which I already have the method insamples.

[ssfrr]

In general I don't think there's harm in allowing the user to specify or omit units, e.g. it seems pedantic not to allow lowpass(200) rather than assuming they mean 200Hz. Is there a reason not to allow both?

One place where the unitful and unit-less arguments have different meanings in SampledSignals is when specifying time, where unitless values are assumed to be in frames: x[1..100, :] is the first 100 frames of all channels and x[0s..0.1s, :] is the first 0.1 seconds.

In general I think the tricky thing about units is that they're very cool and handy as long as they're used consistently, but unless there's a critical mass of code using them then they can feel like a gimmick that adds friction. I agree that if units cancelled out to a unitless number it would smooth out that friction considerably. I'm curious how widely units are used elsewhere in the ecosystem.

So I think I'm onboard with returning unitless values in general, but I'd advocate for accepting unitful or unitless values where possible.

[haberdashPI]

I'm fine with allowing unitless values; certainly the lack of trouble I've had with them being required may be pretty specific to my particular uses.

Thinking about this a little more, it seems like there are three cases of relevance:

1. All units are explicitly specified - here we're on the same page.
2. No units are specified - here I wonder if maybe there should be a warning, but only sparsely. Perhaps just the first time it happens for a given function argument, or just the first time it happens at all.
3. Some unit are specified, some are not - here I definitely think we should at least throw a warning, maybe an error. It seems like this would be most likely due to a mistake.

Thinking through your example of x[1 .. 100]: I would argue there is a unit here; it's frames. It seems slightly redundant to have : and .. mean the same thing for unitless values and if there is any default unit for .. it seems like it could just as well be seconds rather than frames. I actually think I slightly prefer seconds as the default unit for ...

In general I think the tricky thing about units is that they're very cool and handy as long as they're used consistently, but unless there's a critical mass of code using them then they can feel like a gimmick that adds friction. I agree that if units cancelled out to a unitless number it would smooth out that friction considerably. I'm curious how widely units are used elsewhere in the ecosystem.

Good question. Absent further knowledge, it certainly seems safer to make them optional. I think making them optional, but at the very least throwing a warning in the specific case of some units being specified and some not would be good.

[haberdashPI]

I think this can safely be closed, can it not?

[ssfrr]

yup, thanks for the reminder!