flucoma / flucoma-sc

Fluid Corpus Manipulation plugins for Supercollider
BSD 3-Clause "New" or "Revised" License
70 stars 16 forks source link

slicers: change algo/metric select to symbols #103

Closed elgiano closed 2 years ago

elgiano commented 2 years ago

Following up #86, after seeing how the new "select" mechanism work, I think it would be better to select algorithms (in FluidNoveltySlice) and metrics (in FluidOnsetSlice) by passing symbols, rather than using "structs". Just not too have too many different interfaces, that could end up being confusing. I think it's also best to raise this now, since #86 didn't reach a beta yet.

Just to be clear, the difference is:

// using structs
FluidNoveltySlice.ar(in, algorithm: FluidNoveltySlice.mfcc)
// using symbols
FluidNoveltySlice.ar(in, algorithm: \mfcc)

And I think the latter fits better with FluidBufStats.process(select: [\low])

If you decide to adopt this, maybe it's worth changing FluidMLP activation selection as well?

Implementation question: I used lists, maybe you prefer dictionaries? it would be an easy change.

jamesb93 commented 2 years ago

Hey @elgiano that's a good point. I'm certainly not the expert here and @tedmoore is but I can see how different interfaces to achieve similar outcomes is a source of friction.

To me the symbolic way is more terse and clear as you said. I'll leave this to the adults though with my experience scowling at my computer weighing in a little.

tedmoore commented 2 years ago

I agree with @elgiano's suggestion. I'll let @weefuzzy have a look too.

weefuzzy commented 2 years ago

I'd like it if we also still had the option of passing integers, not least for compatibility with existing code. Can't think of an elegant sclang way to do it without brute checking the class of what comes in though.

Don't think a dictionary wins us anything here, because the list is basically immutable and the mapping to values straightforward.

elgiano commented 2 years ago

@weefuzzy we can still pass numbers, of course! You make me realize that I broke support for passing UGens though... should that be supported as well?

Can't think of an elegant sclang way to do it without brute checking the class of what comes in though.

Me neither, so I check isNumber. I think it's ok to do it, since it happens only at SynthDef "build-time". To support UGens I would also check for .isUGen, and that's it

tedmoore commented 2 years ago

@elgiano, we're going to look at putting this in very soon. Thanks!

tedmoore commented 2 years ago

You make me realize that I broke support for passing UGens though... should that be supported as well?

Hi @elgiano, I think this would be good yes. Can you put in the .isUGen check? And then we'll pull it into dev! Thanks for your suggestion and for making the PR!

If you decide to adopt this, maybe it's worth changing FluidMLP activation selection as well?

And yes, I hope this can happen at some point as well. Perhaps I'll port your implementation over there too!

elgiano commented 2 years ago

Doing it! Just one question @tedmoore @weefuzzy: can algorithm/metric be modulated? Or even if they can't, shall we allow control-rate and audio-rate inputs (e.g. in case they get sampled by the algo at init time)?

weefuzzy commented 2 years ago

Doing it! Just one question @tedmoore @weefuzzy: can algorithm/metric be modulated?

Yes, although with different side effects, and only at .kr. In the case of NoveltySlice changing the algorithm means that the sizes of the intermediate representations are changing, so when it changes it implies a total reset (and, currently, this is more expensive than one would like).

tedmoore commented 2 years ago

Thank you @elgiano!

elgiano commented 2 years ago

I think I didn't remove the option to pass ints, did I? Ints (actually floats) are what is going to be passed to the server anyway in the end

On Mon, 30 May 2022, 19:01 Owen Green, @.***> wrote:

I'd like it if we also still had the option of passing integers, not least for compatibility with existing code. Can't think of an elegant sclang way to do it without brute checking the class of what comes in though.

Don't think a dictionary wins us anything here, because the list is basically immutable and the mapping to values straightforward.

— Reply to this email directly, view it on GitHub https://github.com/flucoma/flucoma-sc/pull/103#issuecomment-1141351254, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACNMWRUBDZ5PV5TYEHHDK3VMTYANANCNFSM5W2IGP2Q . You are receiving this because you were mentioned.Message ID: @.***>