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

Review concatenation strategies #31

Open
ssfrr opened this issue Apr 4, 2018 · 6 comments
Open

Review concatenation strategies #31

ssfrr opened this issue Apr 4, 2018 · 6 comments

Comments

@ssfrr
Copy link
Collaborator

ssfrr commented Apr 4, 2018

from @haberdashPI in #29

Bottom line - I could overide Base.vcat, define a new method sequence or do both.

Both mix and envelope seem readily definable for SampleBuf (minus the questions about how to handle promotion I raise [in #30]). However, right now I redefine
Basae.vcat in Sounds to handle concatenation of multiple sounds with
differing numbers of channels. I could do this for SampledSignals as well.

An alternative is that one could define a new method (sequence ??) that can
concateate sounds with different channel counts, and leave Base.vcat
untouched. Things in favor of using vcat are (1) it is an obvious way to
concatenate array-like objects, (2) it has a built in syntax that is visually
comprehensable, and (3) I don't currently envision there being issues with having
vcat behave a little differently for sounds.

An argument against vcat is that it would not be so natural or obvious an
interface when using sound streams. Another potential concern is that automatic
promotion to different channel counts may not make as much sense for all types
of sampled signals. EEG signals should probably not just duplicate or mix
channels (there'd need to be some sort of spatial interpolation... really though that's
an entirely different set of design constraints). Maybe as part of the
formatting object I'm proposing below there is a flag that indicates how
channels should be increased or decreased.

It might even be that both vcat and sequence are
defined, so that users can leverage what they know about vcat easilly, but use
sequence for streams and sounds just as easily.

All of that should be relatively straightforward to implement, so it's mostly a
question of design. I'm inclined to define both, making vcat and sequence
equivalent for SampleBuf objects, but only define sequence for streaming
objects.

@ssfrr
Copy link
Collaborator Author

ssfrr commented Apr 4, 2018

SampleBufs should be drop-in replacement for raw arrays, in the sense that if you've written code for arrays it should also work for SampleBufs. So vcat and hcat should work for concatenating along time and channel, respectively.

That said, for operations that would be an error on Arrays (like vcat for different channel counts or hcat with different lengths), I could see an argument for extending the behavior given that we know a little more about the use-cases. In general I'd expect these to up-mix (concatenating a 1-channel and 3-channel buffer would result in a longer 3-channel buffer)

Streams are an interesting case, where vcat(src1, src2) would create a new stream that would read from src1 until it ends and then start reading from src2, presumably doing the same channel up-mixing that the buffer concatenation does. hcat on streams would read from them both and give the results in parallel. This would extend to sinks as well.

So yeah, I think I'm in favor of extending the use of vcat and hcat to these types of up-mixing combinations. If we were changing Array behavior it would be a different story, but given that these operations would otherwise be errors I think it's reasonable.

@ssfrr
Copy link
Collaborator Author

ssfrr commented Apr 4, 2018

One question about this approach is deciding when to stop, e.g. it seems like the same reasoning would apply to + and *, which could do the same types of automatic extension. I guess we're already assuming that channel extension would happen (because the lower channel-count buffer would get promoted), so it's a question of whether length "promotion" makes sense as well. I'm tempted to include it for convenience, but I'm not sure if at some point it becomes surprising and confusing.

@haberdashPI
Copy link
Contributor

Cool, it sounds like extending the behavior of vcat and hcat for sound seems reasonable to both of us.

I have had similar thoughts about maybe changing mix and envelope to + and *, and ultimately I decided against it in the current API because the additional padding behavior might be surprising. With your suggestion that the padding behavior be configurable, I think it makes even less sense. With mix or envelope it would be very natural to have a keyword argument to specify how to pad. Not as clear what the interface would be to configure padding if the operations were + and *.

I also don't see much of an advantage to + and *: I think the main advantage of using the operators would be succinctness in longer equations (with many operators) but for manipulating sounds, where there will be pipes to lots of other functions, this doesn't help much: in a piping operation x -> x+sound, if anything, is less readable than mix(sound). Another argument against is that in v0.7 of Julia, element-wise + on arrays will be deprecated in favor of .+. I think it would be even more surprising for padding to occur with the broadcasted operator.

@haberdashPI
Copy link
Contributor

Oh, I just realized, I think + and - are being deprecated only for scalar + vector etc.., not vector + vector or matrix + matrix. So I think my comment about that doesn't apply.

@haberdashPI
Copy link
Contributor

As noted in #44, I've grown to like the idea of having + and * (etc...) as additional ways to do mix and amplify. I don't think it would be surprising because + and * are understood to mean something that depends on the specific types: i.e. * always means some kind of matrix-like multiplication, while .* is for element-wise. And it seems pretty logical that + would mean "mix the signals".

@haberdashPI
Copy link
Contributor

Note that #44 would be one way to resolve this issue.

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

2 participants