Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Comparison to AxisArrays #38

Open
ssfrr opened this issue Apr 11, 2018 · 2 comments
Open

Comparison to AxisArrays #38

ssfrr opened this issue Apr 11, 2018 · 2 comments

Comments

@ssfrr
Copy link
Collaborator

ssfrr commented Apr 11, 2018

There's definitely a lot of overlap between SampleBuf and AxisArray. When I originally started SampledSignals.jl, AxisArrays.jl was younger and more experimental and seemed like it might be more general than what I needed. I could provide a simpler API by making the assumption that we're strictly talking about multi-channel regularly-sampled data across time or frequency, with each channel being a column.

That said, I figured it was worth re-visiting this overlap, particularly in light of the work that @haberdashPI has been doing to use the same Interval and Unitful package that AxisArrays uses. Either we figure out that AxisArrays would work well for this use-case or whether we just document why it makes sense to keep doing what we're doing.

Advantages of keeping SampleBuf separate

  • easy syntax for time indexing relative to the buffer start (to get the first second of an AxisArray you need to do foo[atindex(0s..1s, 1)], whereas for a SampleBuf it's just foo[0s..1s])
  • simpler constructor syntax - SampleBuf(data, samplerate) is all you need, vs A = AxisArray(data, Axis{:time}(0s:1s/samplerate:60s), Axis{:channel}([1, 2]))
  • more flexibility adding new methods and behavior, e.g. we have domain, nframes, as well as the variety of resampling/mixing/promotion behaviors we've talked about in Explore promotion mechanism #30 and Review concatenation strategies #31

Advantages of merging with AxisArrays

  • take advantage of the great work and talented developers working on AxisArrays (e.g. not duplicating work on performance tuning)
  • be able to join forces on projects like better DSP.jl integration for filtering
  • less hairy array indexing code to maintain
  • nice features for getting symmetrical windows
  • when working with spectra I think absolute indexing is often more natural, e.g. foospec = barspec[100Hz..5kHz] and then further refine with foospec[1kHz..2kHz]

There might be some middle ground where we have type alias like const SampleBuf{T, N, D, I1, I2} = AxisArray{T, N, D, Tuple{Axis{:time, I1}, Axis{:channel, I2}}} and define more convenient constructors and methods on it, but I worry a bit about the complexity of nested type parameters and aliases being hard to extend and maintain (might not be too bad though...). Also doing anything promotion-related on these types would be type piracy (though less likely to affect anyone because they would have to unwittingly create an AxisArray with :time and :channel axes).

cc @mbauman

@haberdashPI
Copy link
Contributor

haberdashPI commented Apr 11, 2018

I've wondered a bit about this myself. I've leaned away from it because of the type piracy point. I think the more general point is that not all of the things one wants to do with constant-sample-rate signals make sense with the more general AxisArray type.

One possibility, which would still have many of the advantages you list above, is to define SampleBuf and SpectrumBuf as wrappers around an AxisArray. That would allow one to leverage all the great work in AxisArray and still define semantics for sampled signals that are more specific to a particular subset of AxisArrays.

I like the idea of wrapping an AxisArray in the long-term, but I'd be wary of it short term, because there are still some tricky issues in working with AxisArrays. I use them a fair amount in some code where I have 4D arrays that model early cortical auditory responses. I've run into a number of basic functions that don't work very conveniently with AxisArrays, mostly because broadcasting returns plain Arrays (which Julia 0.7 should make fixable) or because of some issue with unitful axis values (which is listed as an issue in AxisArrays, and will presumably be fixed). I also have some issues with the overall ease of use of defining and slicing by a named axes, which I haven't entirely thought through and would like to provide some constructive criticism about once I have.

@mchitre
Copy link

mchitre commented Mar 18, 2020

My two cents worth of thoughts on this:

I experimented with AxisArray as well as AxisIndices quite a bit before settling on using SampleBuf. There are a few things I miss from the AxisArray/AxisIndices approach, especially when dealing with subarrays and strides (keeping track of offsets and effective sample rate changes), but overall the simplicity of SampleBuf made the decision for me for now. I would still keep AxisIndices in view, as it does most of what I need. The only reason I didn't go with it was the fact that indexing with a range causes the index to be collected into an array, and blows up memory usage for large signals. With some effort, that might be fixable in AxisIndices. AxisArray on the other hand loses metadata very easily (as @haberdashPI also notes) in simple operations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants