FluidMFCC arguments inconsistent ordering

[tedmoore]

FluidMFCC is reporting:

WARNING: SCDoc: In /Users/macprocomputer/Desktop/_flucoma/code/flucoma-sc/release-packaging/HelpSource/Classes/FluidMFCC.schelp
  Method *kr has arg named 'maxFFTSize', but doc has 'argument:: maxNumCoeffs'.
WARNING: SCDoc: In /Users/macprocomputer/Desktop/_flucoma/code/flucoma-sc/release-packaging/HelpSource/Classes/FluidMFCC.schelp
  Method *kr has arg named 'maxNumCoeffs', but doc has 'argument:: maxFFTSize'.

because the arguments in the SC class def are in a different order than in the C++, which one can see here:

	*kr { arg in = 0, numCoeffs = 13, numBands = 40, startCoeff = 0, minFreq = 20, maxFreq = 20000, windowSize = 1024, hopSize = -1, fftSize = -1, maxFFTSize = 16384, maxNumCoeffs = 40;
		^this.multiNew('control', in.asAudioRateInput(this), numCoeffs, numBands, startCoeff, minFreq, maxFreq, maxNumCoeffs, windowSize, hopSize, fftSize, maxFFTSize);
	}

Since this is simpler than the FFT-params-fix-business I propose to fix this in the SC class def rather than in the python or elsewhere. Yes, it would be a breaking change for users, but I also think that it won't actually change anyone's code because very rarely would a user set all these params without using keywords anyways.

So, can I make the fix in the SC class def...and if I find similar issues down the road can I take the approach (I'll still certainly PR them for your eyes).

[weefuzzy]

I'm happy enough with this, @tremblap might object though.

From my perspective, these separate max<X> arguments are soon for the chopping block, so I'm relatively unconcerned about swapping the order here.

Off the top of my head as well as this and Chroma (which you've already found), MelBands will have the same thing. And perhaps NoveltySlice

[tedmoore]

Ok, I'll wait for @tremblap to chime in.

From my perspective, these separate max arguments are soon for the chopping block, so I'm relatively unconcerned about swapping the order here.

I agree with this. I often forget that I need to specify this and end up with a KR stream of 40 values, most of which are zeros, then remember to go back and put it in.

[tremblap]

sooooooooo - we carefully thought of that order, with the idea of the more frequent and needed at the front and the less needed at the back. Changing the interface (user) for ease of code maintenance is a dangerous discussion to start with me... now if @weefuzzy says they will disappear soon, I presume for a more dynamic allocation of memory, it all depends how soon.

• really soon, let's not bother moving stuff. Chroma, Mel, HPSS, but also all FFT processes have maxStuff that would be great to disapear.
• by the summer, that is more problematic because we renegade on interface decisions that prove useful - we want important stuff at the top...
• a desiderata, or not happening completely, then I'm even more allergic at moving them.

Does it all make sense? This applies to issue #62 and I reckon an eventual one on Mel and HPSS ;)

[weefuzzy]

A rebuttal 😄 The carefulness of ordering I think diminished as we got to that end of the argument list, because we were dealing with something unfortunately foisted on us by the design. Moreover, I think we got it wrong: the decision was predicated on the idea that changing the maxFFT would be more common than the max, but not so, because of the sucky framework. So, it's not just for ease of maintenance, but actually a marginally less awful choice for something that's intrinsically awful.

Horizon-wise, more towards by the summer than really soon. Which makes it more worth moving them for me, because this represents a small improvement.

[tedmoore]

Changing the interface (user) for ease of code maintenance is a dangerous discussion to start with me...

@tremblap, this makes sense to me. And also I agree with @weefuzzy, for what I think is a slightly adjacent reason. As a user, I never enter all those params in sequence because I never need to use none of the defaults. I often use the defaults of many of them and then use keywords to skip to what I want to specify. So the ordering at the end isn't that relevant because I don't even know what it is. If it changes, I'm like 🤷 .

FWIW a quick change that would somewhat nullify the maxNumCoeffs annoyance is to put this in the class definition before it gets sent to the server:

maxNumCoeffs = maxNumCoeffs ? numCoeffs;

So if max is not specified (is nil) it will default to numCoeffs. This is rather SC-y, not sure how it would interact with the C++.

What is a more short-term concern (at least to me) is that the way things are piped together right now, every time a user opens the FluidChroma (for example) help file, this shows up in the post window (see below). Which feels sloppy--and since we're trying to do the help files revamp having this cleaned up seems like

WARNING: SCDoc: In /Users/macprocomputer/Desktop/_flucoma/code/flucoma-sc/release-packaging/HelpSource/Classes/FluidChroma.schelp
  Method *kr has arg named 'minFreq', but doc has 'argument:: normalize'.
WARNING: SCDoc: In /Users/macprocomputer/Desktop/_flucoma/code/flucoma-sc/release-packaging/HelpSource/Classes/FluidChroma.schelp
  Method *kr has arg named 'maxFreq', but doc has 'argument:: minFreq'.
WARNING: SCDoc: In /Users/macprocomputer/Desktop/_flucoma/code/flucoma-sc/release-packaging/HelpSource/Classes/FluidChroma.schelp
  Method *kr has arg named 'normalize', but doc has 'argument:: maxFreq'.
WARNING: SCDoc: In /Users/macprocomputer/Desktop/_flucoma/code/flucoma-sc/release-packaging/HelpSource/Classes/FluidChroma.schelp
  Method *kr has arg named 'maxFFTSize', but doc has 'argument:: maxNumChroma'.
WARNING: SCDoc: In /Users/macprocomputer/Desktop/_flucoma/code/flucoma-sc/release-packaging/HelpSource/Classes/FluidChroma.schelp
  Method *kr has arg named 'maxNumChroma', but doc has 'argument:: maxFFTSize'.

[tremblap]

ok I prefer to move them than to remove them. the rebuttal is accepted as jousting and I will not argue more. so let's change the order but keep it all because for me the danger of pre-chewing the maxNumCoeffs is that users will then freak if they cannot modulate up and not get the zeros when they modulate down, so all in all just shoving the problem later.

but again, opened to rebuttals - interface discussions are quite important for this project :)

[tedmoore]

The proposal above: maxNumCoeffs = maxNumCoeffs ? numCoeffs; wouldn't remove the option of modulating, instead it would remove the necessity to manually set maxNumCoeffs to the same number as numCoeffs, which I'm pretty sure is going to be the most common use case.

As it currently works, all of the defaults will give the user a 40 channel control stream, 27 of which are just zeros. By making this change the default will give the user 13 channels, or however many they specify in numCoeffs, unless they manually set the maxNumCoeffs, in which case that will be the size of the control stream, so they can modulate.

I think it's a good idea. I don't feel that strongly about it though, just clarifying here.

[tremblap]

I understand this local, CCE specific solution, but it makes a major interface split between the 2 platforms. we need to be able to explain the 0s which will need to be dealt with anyway. I think the simpler approach is to do like max and pd and overload the user of maxNumCoeff to 13 and deal with the problematic interface head-on soon enough until it all disappears.

[tedmoore]

Ok that sounds great. I'll take that route.

[tremblap]

'great' is too kind - I think it is the less worse solution for now :)