Skip to content

Conversation

@elgiano
Copy link
Contributor

@elgiano elgiano commented May 24, 2022

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
Copy link
Member

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
Copy link
Member

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

@weefuzzy
Copy link
Member

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
Copy link
Contributor Author

elgiano commented May 30, 2022

@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
Copy link
Member

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

@tedmoore
Copy link
Member

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
Copy link
Contributor Author

elgiano commented Jun 16, 2022

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
Copy link
Member

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 tedmoore added this to the Version 1.0 milestone Jun 17, 2022
@elgiano elgiano force-pushed the topic/slicers-select-algo branch from e8dc0bc to 4e7cedb Compare June 17, 2022 17:37
@tedmoore tedmoore merged commit 5496cf5 into flucoma:dev Jun 29, 2022
@tedmoore
Copy link
Member

Thank you @elgiano!

@jamesb93 jamesb93 mentioned this pull request Jun 30, 2022
jamesb93 pushed a commit that referenced this pull request Jun 30, 2022
* ignore more varieties of build folder

* deal with some warnings

* FluidWaveform 'lineWidth' argument now also affects feature lines

* add back the nmf-jit-classifier example

* novelty interface change in example

* resizable and layoutable guis (#83)

* resizable and layoutable guis

* FluidWaveform: rename 'win' to 'parent'

* FluidWaveform/FluidPlotter: update help

* FluidWaveform/Plotter: make views before forking

This way views are immediately available upon creation,
for example to be added to layouts.
Views are still correctly updated with data from within the fork,
whenever they are ready.

* Thanks @elgiano! + a few small edits

Co-authored-by: Ted Moore <[email protected]>

* slicers: add enums for algorithms (#86)

* typo

* FluidBufNMF class: add resynthMode argument

* add PCA whitening parameter (#65)

* add PCA whitening parameter

* FluidPCA: Ensure whiten parameter is sent to kr query UGen

Co-authored-by: Gerard <[email protected]>
Co-authored-by: weefuzzy <[email protected]>

* Enhance/optional message args (#77)

* optional args: sc wrapper updates

* optional args: KDTree try out

* Enhance/choices param (#78)

* NRTWrapper: Add choices param (long <-> bitset)

* Update `FluidBufStats` with `select` control

* BufStats class: Fix bitfield for `select` and warn on duplicate items

* Update SpectralShape classes for new param

* `PCA.sc`: add batch `inverseTranform` method

* Wrapper: integer sign warnings

* Enhance/max params (#93)

* CMake: Centralise C++ version and set to 17

* Wrapper: handle new LongRuntimeMax param type

* POC for new LongRuntimeMax param with MFCC numCoeffs

* Wrapper: Make MSVC happy about constexpr lambda capture

* All scalers: replace 'invert' parameter with `inverseTransform` messages

* Wrapper: Work around 32 char limit for plugin commands

If too long, remove vowels. 
Sorry. 
Better ideas welcome

* fix #96

* typo

* Update SC classes for new style `max<X>` parameters

* SpectralShape SC class: maxFFT

* RealTime wrapper: play it safer with output channel count

This really relies on the SC class being correct, but then everything 
ultimately does...

* RT FFT Object SC Classes: Provide maxFFTSize default

* BufSTFT SC class: Add maxFFT (now needed due to core type change)

* Add select param to Loudness and Pitch SC clases (#101)

* Wrapper: workaround scsynth 32 char cmd length limit with extra dispatch layer 

also avoids need for formerly truncated plugin names in some cases

* removed invert from scalers class definitions (#102)

* Enhance/generate stubs (#104)

* CMake: generate .cpp stubs

* Remove old cpp stubs

* Ensure correct MSVC runtime by default

* CMake: invoke docs properly

* CMake: Tidy up

* CMake: Tidy up

* CMake: typo

* CI: Update nightly

* CI: remove lingering references to docs job

* CMake: belatedly add branch selection for flucoma deps upon which CI relies

* CMake: Actually commit important code for best collaborative results

* CMake: This file is now redundant, in fact

* cmake: missing slash in install

* bufnmf: added the maxFFTsize parameter in the server call

* FluidStats: Change where output Array reshape happens to keep SynthDescLib happy

* FluidBufNNDSVD: maxfftsize now needed in server call, or booooom

fixes #161

* BufNMFCross: Needs MaxFFTSize

* BufNNDSVD: Ensure activations buffer is queried at finish

* FluidBufToKr ensure that numFrames is an int (not a float)

* Added *(Buf)Feature objects to guide (and deleted old guide)

NNDSVD --> NMFSeed in Guide

fixed bad links in Guide

* change interface and file name (#113)

* hidden --> hiddenLayers in class definition (#114)

* reordered some max<X> arguments

* change interp to interpolation in nmfmorph class (#115)

* Feature/skmeans (#66)

* add PCA whitening parameter

* add FluidSKMeans

* SKMeans correction

* added RT query

* <fit>transform<point> -> <fit>encode<point>

* added to overview

Co-authored-by: Gerard <[email protected]>
Co-authored-by: tremblap <[email protected]>

* [CI] Update Release Workflow (#118)

* cleanup nightly.yaml

* use new release style

* refactor release

* remove workflow dispatch variables

* interface changes in 8c

* knearest interface change in 10a

* waveform help nmf interface change

* capitalise beatRemember

* two more changes of interface

* typo

* sign binaries

* add -nightly affix

* sign releases too

* enforce concurrency of jobs

* UMAP kr method should not allow user to pass numDimensions

* slicers: change algo/metric select to symbols (#103)

* slicers: change algo/metric selection to symbols

* slicers: algo/metric accept UGen

* FluidDataSetWr example code (#124)
@elgiano
Copy link
Contributor Author

elgiano commented Oct 11, 2022 via email

@elgiano elgiano deleted the topic/slicers-select-algo branch July 19, 2023 23:25
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

Successfully merging this pull request may close these issues.

4 participants