grame-cncm / faustlibraries

The Faust libraries
https://faustlibraries.grame.fr
188 stars 61 forks source link

trouble adding compressors #3

Closed magnetophon closed 1 year ago

magnetophon commented 6 years ago

I worked on adding the compressors, but when testing, I keep getting either:

compressors.lib : 613 : ERROR : undefined symbol : si

or

BoxIdent[si] is defined here : RMS_FBcompressor_peak_limiter.dsp:12

Here's the content of my test file, RMS_FBcompressor_peak_limiter.dsp:

declare author "Bart Brouns";
declare name "RMS_FBcompressor_peak_limiter";
declare version "1.2";
declare license "GPLv3";

// import("compressors.lib");
// import("stdfaust.lib");

ba = library("basics.lib");
ma = library("maths.lib");
co = library("compressors.lib");
// si = library("signals.lib");

process =
co.RMS_FBcompressor_peak_limiter_N_chan(strength,threshold,thresholdLim,attack,release,knee,link,meter,2);

maxRelTime = co.rmsMaxSize/sr;
sr = 44100;

    comp_group(x) = vgroup("COMPRESSOR  [tooltip: Reference: http://en.wikipedia.org/wiki/Dynamic_range_compression]", x);

    meter_group(x)  = comp_group(vgroup("[0]", x));
    knob_group(x)  = comp_group(hgroup("[1]", x));

    checkbox_group(x)  = meter_group(hgroup("[0]", x));

    cbp = checkbox_group(checkbox("[0] Bypass  [tooltip: When this is checked, the compressor has no effect]"));
    maxGR = -100;
    meter = _<:(_, (ba.linear2db:max(maxGR):meter_group((hbargraph("[1][unit:dB][tooltip: gain reduction in dB]", maxGR, 0))))):attach;

    ctl_group(x)  = knob_group(hgroup("[3] Compression Control", x));

    strength = ctl_group(hslider("[0] Strength [style:knob]
      [tooltip: A compression Strength of 0 means no gain reduction and 1 means full gain reduction]",
      1, 0, 8, 0.01));

    duck_strength =
        ctl_group(hslider("[-1] Duck Strength [style:knob]
          [tooltip: A compression Strength of 0 means no gain reduction and 1 means full gain reduction]",
          1, 0, 8, 0.01));

    expand_strength =
        ctl_group(hslider("[0] Expand Strength [style:knob]
          [tooltip: A compression Strength of 0 means no gain reduction and 1 means full gain reduction]",
          1, 0, 8, 0.01));

    threshold = ctl_group(hslider("[1] Threshold [unit:dB] [style:knob]
      [tooltip: When the signal level exceeds the Threshold (in dB), its level is compressed according to the Strength]",
      0, maxGR, 10, 0.1));

    knee = ctl_group(hslider("[2] Knee [unit:dB] [style:knob]
      [tooltip: soft knee amount in dB]",
      6, 0, 30, 0.1));

    HPfreq =
        ctl_group(hslider("[3] HP freq [scale:log] [style:knob]
          [tooltip: cutoff frequency of the sidechain fi.highpass filter]",
          20, 20, 10000, 1));

    LPfreq =
        ctl_group(hslider("[4] LP freq [scale:log] [style:knob]
          [tooltip: cutoff frequency of the sidechain fi.lowpass filter]",
          10000, 20, 10000, 1));

    SClisten = knob_group(checkbox("SC"));

    env_group(x)  = knob_group(hgroup("[4] Compression Response", x));

    attack = env_group(hslider("[1] Attack [unit:ms] [style:knob] [scale:log]
      [tooltip: Time constant in ms (1/e smoothing time) for the compression gain to approach (exponentially) a new lower target level (the compression `kicking in')]",
      0.1, 0.1, 1000, 0.01)-0.1) : *(0.001) ;
    // The actual attack value is 0.1 smaller than the one displayed.
    // This is done for hard limiting:
    // You need 0 attack for that, but a log scale starting at 0 is useless

    release = env_group(hslider("[2] Release [unit:ms] [style: knob] [scale:log]
      [tooltip: Time constant in ms (1/e smoothing time) for the compression gain to approach (exponentially) a new higher target level (the compression 'releasing')]",
      100, 1, maxRelTime*1000, 0.1)) : *(0.001) : max(1/ma.SR);

    prePost = env_group(checkbox("[3] slow/fast  [tooltip: Unchecked: log  domain return-to-threshold detector
      Checked: linear return-to-fi.zero detector]")*-1)+1;

    link = env_group(hslider("[4] link [style:knob]
      [tooltip: 0 means all channels get individual gain reduction, 1 means they all get the same gain reduction]",
      1, 0, 1, 0.01));

    FBFF = env_group(hslider("[5] feed-back/forward [style:knob]
      [tooltip: fade between a feedback and a feed forward compressor design]",
      1, 0, 1, 0.01));

    lim_group(x)  = knob_group(hgroup("[5] Limiter [tooltip: It's release time is the minimum of the attack and release of the compressor,
      and it's knee is half that of the compressor]", x));

    thresholdLim = lim_group(hslider("[9] Threshold [unit:dB] [style:knob]
      [tooltip: The signal level never exceeds this threshold]",
      0, -30, 10, 0.1));

    makeupgain = comp_group(hslider("[6] Makeup Gain [unit:dB]
      [tooltip: The compressed-signal input level is increased by this amount (in dB) to make up for the level lost due to compression]",
      0, 0, maxGR*-1, 0.1)) : ba.db2linear;

@josmithiii Could you have a look at the naming? I declared your compressors as being superseded by mine, I hope you don't see that as rude!

@rmichon I noticed you are not on the list of people "watching" this repo, so here's a ping.

rmichon commented 6 years ago

Interesting... I've see this error before and it basically tells you that there's something broken in the libraries without really being very specific haha. So I'm afraid the only way to identify the origin of the problem is to comment things out step by step. I wish I could give you a better answer haha. At least you know that the issue must come from the change you've made.

Here are a few other things I've noticed while quickly looking at your commit:

declare author "Bart Brouns";
declare license "GPLv3";

to compressors.lib because it will be applied to all the functions declared in that library (including the one written by @josmithiii). @orlarey, @josmithiii, and myself were talking about implementing a declare that would be function-specific but we haven't done it yet. For now, you should put your name, license, and copyright outside of the comment section that ends up in the doc:

// Doc section
//-----------------------------------------------------
// Author: You
// License: STK-4.3

until we get something better...

rmsMaxSize = 2:pow(17); // highest usable for faust2lv2
sr = 44100;
maxRelTime = rmsMaxSize/sr;

especially sr that might create confusions with ma.SR. I would move those inside the function using them (using with{}) even if it implied redefining them internally a few times. Also, why do you need sr=44100? Does it mean that some functions were designed to work at a specific sampling rate? Why not use ma.SR? rmsMaxSize could be defined as a global variable of compressors.lib but it should appear in the doc in that case...

magnetophon commented 6 years ago

I found the problem: basics.lib was using si. without declaring it. When you remove the definition of si in basics.lib, you get: BoxIdent[si] is defined here : routes.lib:39, and not undefined symbol : si Would it be possible to get a better error message in that situation?

I fixed most of the other issues, but I'm still looking for a good solution regarding rmsMaxSize.

rmsMaxSize is the main factor in CPU usage, so maybe I should make it a parameter of the RMS compressors? If not, I could move it inside RMS_compression_gain_mono, but that would not be ideal, because you need it to calculate the max value of the release slider. A third option would be to leave it where it is, and document it.

Am I missing any options?

Another issue I'd like to fix is that the maximum release time depends on the sample rate, but a slider's maximum value needs to be known at compile time. Any ideas?

Finally: should I add some compressors to demos.lib? If so: in addition to the current ones, or instead of the current ones?

Are you happy with the rest of my code and comments?

magnetophon commented 6 years ago

@josmithiii In an effort to keep the discussion of each topic in the correct place, I'll reply to this here.

As requested, here is my compressors.lib. It uses some functions I added to basics.lib.

Thanks for having a look!

Once you are OK with the content of those, I'll do a pull request.

josmithiii commented 6 years ago

Perfect, thanks

I know this sounds lame, but after 5 minutes trying I cannot figure out how to download the files for comparing in Emacs. What is the preferred method for this?

On Sun, Apr 1, 2018 at 2:40 PM, Bart Brouns notifications@github.com wrote:

@josmithiii https://github.com/josmithiii In an effort to keep the discussion of each topic in the correct place, I'll reply to this https://github.com/grame-cncm/faustlibraries/pull/4#issuecomment-377813380 here.

As requested, here is my compressors.lib https://github.com/magnetophon/faustlibraries/blob/master/compressors.lib . It uses some functions I added to basics.lib https://github.com/magnetophon/faustlibraries/blob/master/basics.lib.

Thanks for having a look!

Once you are OK with the content of those, I'll do a pull request.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/grame-cncm/faustlibraries/issues/3#issuecomment-377818835, or mute the thread https://github.com/notifications/unsubscribe-auth/ACGVFU_42iAdLQVWujTjWBfzTyg8TWY0ks5tkUlDgaJpZM4S55YN .

--

Julius O. Smith III jos@ccrma.stanford.edu Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/ http://ccrma.stanford.edu/

magnetophon commented 6 years ago

You can either do

git clone https://github.com/magnetophon/faustlibraries/

or you can go to the links in my previous posts, right click the Raw button, and select save.

josmithiii commented 6 years ago

Ok thanks - this worked: git clone https://github.com/magnetophon/faustlibraries faustlibraries-bart

On Sun, Apr 1, 2018 at 3:43 PM, Bart Brouns notifications@github.com wrote:

You can either do

https://github.com/magnetophon/faustlibraries/

or you can go to the links in my previous posts, right click the raw button, and select save.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/grame-cncm/faustlibraries/issues/3#issuecomment-377822582, or mute the thread https://github.com/notifications/unsubscribe-auth/ACGVFRU6hql5PlazT2fZbOFi-sUPdSahks5tkVgigaJpZM4S55YN .

--

Julius O. Smith III jos@ccrma.stanford.edu Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/ http://ccrma.stanford.edu/

rmichon commented 6 years ago

@magnetophon it all looks good to me. I think there might a few changes needed in the documentation of some functions, but I can take care of that, shouldn't take too long ;). @josmithiii, once you say you're OK with @magnetophon contributions, can @magnetophon do a PR? Thanks for your hard work @magnetophon !

magnetophon commented 6 years ago

@rmichon Cool. What should I do with rmsMaxSize?

josmithiii commented 6 years ago

Today I looked closely at Bart's basics.lib and everything looks great.

It was useful to learn about the inputs(b) primitive giving the number of signals in b.

On Sun, Apr 1, 2018 at 7:34 PM, Romain notifications@github.com wrote:

@magnetophon https://github.com/magnetophon it all looks good to me. I think there might a few changes needed in the documentation of some functions, but I can take care of that, shouldn't take too long ;). @josmithiii https://github.com/josmithiii, once you say you're OK with @magnetophon https://github.com/magnetophon contributions, can @magnetophon https://github.com/magnetophon do a PR? Thanks for your hard work @magnetophon https://github.com/magnetophon !

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/grame-cncm/faustlibraries/issues/3#issuecomment-377840081, or mute the thread https://github.com/notifications/unsubscribe-auth/ACGVFZUkd5Rq5ULGcvlIDtCHq5Y_7Ck3ks5tkY4zgaJpZM4S55YN .

--

Julius O. Smith III jos@ccrma.stanford.edu Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/ http://ccrma.stanford.edu/

josmithiii commented 6 years ago

The only disappointment I noted was use of GPL instead of MIT or the like, since I avoid use of GPL code in my consulting work.

On Mon, Apr 2, 2018 at 5:37 PM, Julius Smith jos@ccrma.stanford.edu wrote:

Today I looked closely at Bart's basics.lib and everything looks great.

It was useful to learn about the inputs(b) primitive giving the number of signals in b.

  • Julius

On Sun, Apr 1, 2018 at 7:34 PM, Romain notifications@github.com wrote:

@magnetophon https://github.com/magnetophon it all looks good to me. I think there might a few changes needed in the documentation of some functions, but I can take care of that, shouldn't take too long ;). @josmithiii https://github.com/josmithiii, once you say you're OK with @magnetophon https://github.com/magnetophon contributions, can @magnetophon https://github.com/magnetophon do a PR? Thanks for your hard work @magnetophon https://github.com/magnetophon !

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/grame-cncm/faustlibraries/issues/3#issuecomment-377840081, or mute the thread https://github.com/notifications/unsubscribe-auth/ACGVFZUkd5Rq5ULGcvlIDtCHq5Y_7Ck3ks5tkY4zgaJpZM4S55YN .

--

Julius O. Smith III jos@ccrma.stanford.edu Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/ http://ccrma.stanford.edu/

--

Julius O. Smith III jos@ccrma.stanford.edu Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/ http://ccrma.stanford.edu/

josmithiii commented 6 years ago

If I understand correctly, rmsMaxSize sets the maximum number of samples in the sliding rectangular window, and the issue is simply where to put it. I think it should be an argument passed to every function all the way down to maxn in slidingReduce. I would also make it the first argument like delay() and friends do. That way one can absorb it into the function name if desired:

delay8K = delay(8192);

The slidingReduce primitive added to basics.lib is a very cool construct (and not easy to write!) which achieves log2(N) complexity for performing N operations by doing them in a tree structure. However, note that in the present situation, complexity O(1) is available, e.g.,

slidingSumN(n,maxn) = slidingReduce(+,n,maxn,0);

can instead be

slidingSum(n) = _ <: fi.integrator, fi.integrator@int(max(0,n)) :> -; // 0 guard optional, but I would keep it

Finally, note that slidingSumN and slidingMeanN have N in their names while other analogous functions do not. I would omit the N whenever the N=1 case is not an often-used case that would benefit from getting the "no N" version name instead of having to write functionName(1,...) all the time. This worked out well for maximize and bypass, for example, in basics.lib.

I still need to look at the compressors, but I'm sure they'll be fine as long as old code does not break. If the licenses can be changed to STK-4.2 or MIT, etc., I will rewrite and/or deprecate anything I wrote, as appropriate, in favor of the new functions. (I think of STK-4.2 as "MIT + please share your enhancements".) Otherwise I will need to maintain a "parallel universe" of STK-4.2 compressors and such.

Thanks Bart for these solid contributions!

On Mon, Apr 2, 2018 at 5:41 PM, Julius Smith jos@ccrma.stanford.edu wrote:

The only disappointment I noted was use of GPL instead of MIT or the like, since I avoid use of GPL code in my consulting work.

  • Julius

On Mon, Apr 2, 2018 at 5:37 PM, Julius Smith jos@ccrma.stanford.edu wrote:

Today I looked closely at Bart's basics.lib and everything looks great.

It was useful to learn about the inputs(b) primitive giving the number of signals in b.

  • Julius

On Sun, Apr 1, 2018 at 7:34 PM, Romain notifications@github.com wrote:

@magnetophon https://github.com/magnetophon it all looks good to me. I think there might a few changes needed in the documentation of some functions, but I can take care of that, shouldn't take too long ;). @josmithiii https://github.com/josmithiii, once you say you're OK with @magnetophon https://github.com/magnetophon contributions, can @magnetophon https://github.com/magnetophon do a PR? Thanks for your hard work @magnetophon https://github.com/magnetophon !

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/grame-cncm/faustlibraries/issues/3#issuecomment-377840081, or mute the thread https://github.com/notifications/unsubscribe-auth/ACGVFZUkd5Rq5ULGcvlIDtCHq5Y_7Ck3ks5tkY4zgaJpZM4S55YN .

--

Julius O. Smith III jos@ccrma.stanford.edu Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/ http://ccrma.stanford.edu/

--

Julius O. Smith III jos@ccrma.stanford.edu Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/ http://ccrma.stanford.edu/

--

Julius O. Smith III jos@ccrma.stanford.edu Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/ http://ccrma.stanford.edu/

magnetophon commented 6 years ago

@josmithiii

Thanks for the review!

I like the GPL much more than the free-er licenses, though I understand it's not ideal for your purposes. I will think about it.

Yes, it's probably best to make rmsMaxSize an argument, will do.

Thanks for the compliment on slidingReduce; it means a lot to me! Is this an existing algorithm? Do you know it's name?

Your slidingSum is much more elegant. However, when I do:

process(x) =
  (x:abs:slidingSum(n))-(x:abs:ba.slidingSumN(n,maxn)):hbargraph("dif", -5, 5);
slidingSum(n) = _ <: fi.integrator, fi.integrator@int(max(0,n)) :> -;
maxn = pow(2,21);

the hbargraph moves, especially when I move the slider. Any idea why this is?

I'll remove the N from the names.

josmithiii commented 6 years ago

Hi Bart,

I am not aware of a special name for organizing a string of computations into a tree structure to maximize reuse, although there are special examples such as "Fast Fourier Transform" (FFT). It also comes up often in parallel computing since you can start the base of the tree fully in parallel. For example, a length N dot product can happen in log2(N) steps (adds).

By "guard" I meant the "max(0)" step, which guards against negative delays. Such a guard is useful when hooking up oscillating networks and such to the length parameter, but unnecessary in normal use. Ideally the Faust compiler will remove the compare to 0 when it can prove that the signal is nonnegative. I've sometimes added 'p' to the name of a function when it is "protected" in this sort of way (e.g., tf2np = transfer-function of order 2, normalized ladder, and protected to be stable no matter what coefficients you give it).

I see a small jitter on hbargraph as well (Qt, Mac). I think it means that the algorithms are not equivalent for a random (mic) input, so I guess there is further debugging to do.

On Tue, Apr 3, 2018 at 3:32 AM, Bart Brouns notifications@github.com wrote:

@josmithiii https://github.com/josmithiii

Thanks for the review!

I like the GPL much more than the free-er licenses, though I understand it's not ideal for your purposes. I will think about it.

Yes, it's probably best to make rmsMaxSize an argument, will do.

Thanks for the compliment on slidingReduce; it means a lot to me! Is this an existing algorithm? Do you know it's name?

Your slidingSum is much more elegant. However, when I do:

process(x) = (x:abs:slidingSum(n))-(x:abs:ba.slidingSumN(n,maxn)):hbargraph("dif", -5, 5); slidingSum(n) = _ <: fi.integrator, fi.integrator@int(max(0,n)) :> -; maxn = pow(2,21);

the hbargraph moves, especially when I move the slider. Any idea why this is?

What do you mean by:

guard optional, but I would keep it

I'll remove the N from the names.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/grame-cncm/faustlibraries/issues/3#issuecomment-378204505, or mute the thread https://github.com/notifications/unsubscribe-auth/ACGVFdJMEzeWDYGQ-NwZpmAiwguS0Ch1ks5tk0-lgaJpZM4S55YN .

--

Julius O. Smith III jos@ccrma.stanford.edu Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/ http://ccrma.stanford.edu/

magnetophon commented 6 years ago

A minute after I asked about the guard, I realized what you meant and removed the question, but since you interact with github trough mail, it was to late...

Where would one start debugging? I tried comparing both versions to:

bruteForceSum(n,x) = x:seq(i, maxn, _+(x@(i+1)*((i+1)<n)));

using

maxn = pow(2,10);

but neither give the same result. I'm a bit lost as to what to do next.

Also, I thought fi.integrator would run out of bits at some point, but some testing and some googling made clear that that is unlikely to happen, even with single precision floating point. Is that a correct train of thought?

josmithiii commented 6 years ago

Ok, it sounds like I should always click on the pull-request link to get latest info.

My tests also come out clean, e.g.:

// trunningsum.dsp // Usage: faust2plot trunningsum.dsp; trunningsum

import("stdfaust.lib"); bart = library("basics-bart.lib");

n = 12; signal = 1; bruteForceSum(n,x) = x:seq(i, maxn, +(x@(i+1)*((i+1)<n))); process = signal <: bruteForceSum(n), slidingSum(n), bart.slidingSumN(n,maxn); // :> - : hbargraph("dif", -2, 2); slidingSum(n) = <: fi.integrator, fi.integrator@int(max(0,n)) :> -; maxn = pow(2,10);

It is true that fi.integrator will eventually run into numerical trouble when there is a persistent dc component. One easy guard against that is to replace fi.integrator by fi.pole(p) where p=1-1/maxn, e.g., gives a "leaky integrator" that is guaranteed to reach a finite steady-state gain for dc. The completely clean thing to use here is Truncated Infinite Impulse Response (TIIR) filters. However, this issue exists in many places throughout the libraries, so I haven't worried about it too much. When someone actually runs into trouble, I suppose we can start a new line of library functions that are numerically stable "forever". This has not yet come up for me, or in any issue reports I've seen.

On Tue, Apr 3, 2018 at 1:10 PM, Bart Brouns notifications@github.com wrote:

A minute after I asked about the guard, I realized what you meant and removed the question, but since you interact with github trough mail, it was to late...

Where would one start debugging? I tried comparing both versions to:

bruteForceSum(n,x) = x:seq(i, maxn, _+(x@(i+1)*((i+1)<n)));

using

maxn = pow(2,10);

but neither give the same result. Your algorithm only very occasionally differs from the brute force one though. I'm a bit lost as to what to do next.

Also, I thought fi.integrator would run out of bits at some point, but some testing and some googling made clear that that is unlikely to happen, even with single precision floating point. Is that a correct train of thought?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/grame-cncm/faustlibraries/issues/3#issuecomment-378380026, or mute the thread https://github.com/notifications/unsubscribe-auth/ACGVFR-t0EGxUElrgqQrMXdiMUbanJF3ks5tk9cxgaJpZM4S55YN .

--

Julius O. Smith III jos@ccrma.stanford.edu Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/ http://ccrma.stanford.edu/

magnetophon commented 6 years ago

My tests don't come out clean: I can't get the 3 versions to agree, when the input is random.

josmithiii commented 6 years ago

In case it is not clear, I would simply use fi.integrator and do only signal-processing-level testing. I would test and debug slidingReduce only when I had a winning use-case for it. I think the corner-case we have here must be in there somewhere, and if the new application doesn't care about it, I would let it be. That's the FOSS way! :-)

On Tue, Apr 3, 2018 at 1:10 PM, Bart Brouns notifications@github.com wrote:

A minute after I asked about the guard, I realized what you meant and removed the question, but since you interact with github trough mail, it was to late...

Where would one start debugging? I tried comparing both versions to:

bruteForceSum(n,x) = x:seq(i, maxn, _+(x@(i+1)*((i+1)<n)));

using

maxn = pow(2,10);

but neither give the same result. Your algorithm only very occasionally differs from the brute force one though. I'm a bit lost as to what to do next.

Also, I thought fi.integrator would run out of bits at some point, but some testing and some googling made clear that that is unlikely to happen, even with single precision floating point. Is that a correct train of thought?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/grame-cncm/faustlibraries/issues/3#issuecomment-378380026, or mute the thread https://github.com/notifications/unsubscribe-auth/ACGVFR-t0EGxUElrgqQrMXdiMUbanJF3ks5tk9cxgaJpZM4S55YN .

--

Julius O. Smith III jos@ccrma.stanford.edu Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/ http://ccrma.stanford.edu/

josmithiii commented 6 years ago

My tests don't come out clean: I can't get the 3 versions to agree, when the input is random.

Our emails crossed in the ether, but my previous email still holds for me.

Do any two of the three agree with each other?

On Wed, Apr 4, 2018 at 5:17 PM, Bart Brouns notifications@github.com wrote:

My tests don't come out clean: I can't get the 3 versions to agree, when the input is random.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/grame-cncm/faustlibraries/issues/3#issuecomment-378784425, or mute the thread https://github.com/notifications/unsubscribe-auth/ACGVFb_4_zOS-Ju9jjIMkwPdUIGTcQ0lks5tlWKrgaJpZM4S55YN .

--

Julius O. Smith III jos@ccrma.stanford.edu Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/ http://ccrma.stanford.edu/

magnetophon commented 6 years ago

I would test and debug slidingReduce only when I had a winning use-case for it.

My use case for slidingReduce is slidingMin, in LazyLimiter

Do any two of the three agree with each other?

No, I can't get any 2 to agree.

josmithiii commented 6 years ago

That is very interesting --- an irresistible debugging challenge I would say :-)

My "signal processing answer" for slidingMin (also O(1) complexity) would be

runningMin = min ~ _ ; slidingMin(n) = min ~ *(1.0-(1.0/max(1.0,n))); // sets exponentially fading-memory exponential time-constant near n samples

I suppose this should be a special case of exponentiallyFadingReduce(op, ...) with op = 'min'.

For op = 'max', we get the classic amplitude envelope follower an.amp_follower(n/ma.SR) for nonnegative signals.

On Wed, Apr 4, 2018 at 5:33 PM, Bart Brouns notifications@github.com wrote:

I would test and debug slidingReduce only when I had a winning use-case for it.

My use case for slidingReduce is slidingMin, in LazyLimiter https://github.com/magnetophon/LazyLimiter/blob/master/LazyLimiter.lib#L110

Do any two of the three agree with each other?

No, I can't get any 2 to agree.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/grame-cncm/faustlibraries/issues/3#issuecomment-378786629, or mute the thread https://github.com/notifications/unsubscribe-auth/ACGVFbbHRXsT9ay2E7Uz03ooEkxd-mGiks5tlWZagaJpZM4S55YN .

--

Julius O. Smith III jos@ccrma.stanford.edu Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/ http://ccrma.stanford.edu/

magnetophon commented 6 years ago

That's also an interesting construct, but not suitable for my use-case.

The neat thing about LazyLimiter is that it has 0% distortion even when limiting low frequency sine-waves, yet when limiting a transient, it has a quick release time. It does this by looking ahead 186ms; if the current gain-reduction is needed again within the window, it doesn't release (hence the 0% distortion), otherwise it does. So for LazyLimiter, I really need the actual minimum of the last n samples, no exponential window trickery.

Do you have any ideas for debugging slidingReduce?

josmithiii commented 6 years ago

Do you have any ideas for debugging slidingReduce?

I would just do the usual thing of finding the minimum failing case on a repeatable input signal (such as no.noise) and drill down on the C++, maybe in gdb.

On Thu, Apr 5, 2018 at 3:16 PM, Bart Brouns notifications@github.com wrote:

That's also an interesting construct, but not suitable for my use-case.

The neat thing about LazyLimiter is that it has 0% distortion even when limiting low frequency sine-waves, yet when limiting a transient, it has a quick release time. It does this by looking ahead 186ms; if the current gain-reduction is needed again within the window, it doesn't release (hence the 0% distortion), otherwise it does. So for LazyLimiter, I really need the actual minimum of the last n samples, no exponential window trickery.

Do you have any ideas for debugging slidingReduce?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/grame-cncm/faustlibraries/issues/3#issuecomment-379093307, or mute the thread https://github.com/notifications/unsubscribe-auth/ACGVFbgqQnCskhWssLOGq4madvuFzGtxks5tlpeygaJpZM4S55YN .

--

Julius O. Smith III jos@ccrma.stanford.edu Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/ http://ccrma.stanford.edu/

josmithiii commented 6 years ago

Here is my minimum failing case at the moment. Very interesting:

// trunningsum.dsp // Usage: // faust2plot trunningsum.dsp; trunningsum

import("stdfaust.lib"); bart = library("basics-bart.dsp");

//signal = 1; // ok //signal = ba.time; // ok signal = no.noise; maxn = pow(2,5); n = 3; bruteForceSum(n,x) = x:seq(i, n-1, +(x@(i+1))); slidingSum(n) = <: fi.integrator, fi.integrator@int(max(0,n)) : -; process = signal <: ,bruteForceSum(n), slidingSum(n), bart.slidingSumN(n,maxn); // :> - : hbargraph("dif", -2, 2);

-- Here is my output:

faustout = [ ... 5.74859e-06 5.74859e-06 5.74859e-06 5.74859e-06; ... -0.344846 -0.34484 -0.34484 -0.34484; ... -0.695186 0.959974 -1.04003 0.959974; ... -0.325039 0.634929 -1.36507 0.634929; ... 0.106768 -0.913457 -0.913457 -0.913457; ... -0.483426 -0.701696 -0.701697 -0.701696; ... 0.489666 0.113009 0.113009 0.113009; ... -0.397528 -0.391287 -0.391287 -0.391287; ... -0.630045 -0.537907 -0.537907 -0.537907; ... 0.256667 -0.770906 -0.770906 -0.770906; ... -0.625818 -0.999196 -0.999196 -0.999196; ... 0.825585 0.456434 0.456434 0.456434; ... -0.82728 -0.627513 -0.627513 -0.627513; ... 0.297812 0.296117 0.296117 0.296117; ... 0.643531 0.114062 0.114062 0.114062; ... 0.789655 -0.269003 1.731 -0.269003; ... ];

Note that 0.297812 + 0.643531 + 0.789655 = 1.731 and 0.297812 + 0.643531 -0.82728 = 0.11406.

Curious! Unfortunately, I had only a few minutes to go after this. I might be overlooking something really simple.

On Sun, Apr 8, 2018 at 11:45 PM, Julius Smith jos@ccrma.stanford.edu wrote:

Do you have any ideas for debugging slidingReduce?

I would just do the usual thing of finding the minimum failing case on a repeatable input signal (such as no.noise) and drill down on the C++, maybe in gdb.

  • Julius

On Thu, Apr 5, 2018 at 3:16 PM, Bart Brouns notifications@github.com wrote:

That's also an interesting construct, but not suitable for my use-case.

The neat thing about LazyLimiter is that it has 0% distortion even when limiting low frequency sine-waves, yet when limiting a transient, it has a quick release time. It does this by looking ahead 186ms; if the current gain-reduction is needed again within the window, it doesn't release (hence the 0% distortion), otherwise it does. So for LazyLimiter, I really need the actual minimum of the last n samples, no exponential window trickery.

Do you have any ideas for debugging slidingReduce?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/grame-cncm/faustlibraries/issues/3#issuecomment-379093307, or mute the thread https://github.com/notifications/unsubscribe-auth/ACGVFbgqQnCskhWssLOGq4madvuFzGtxks5tlpeygaJpZM4S55YN .

--

Julius O. Smith III jos@ccrma.stanford.edu Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/ http://ccrma.stanford.edu/

--

Julius O. Smith III jos@ccrma.stanford.edu Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/ http://ccrma.stanford.edu/

magnetophon commented 6 years ago

@josmithiii Thanks for the debugging effort. I wanted to get back to you with a useful result, but have not been able to do so so far. Just as a heads up, I won't have time for this for a while, sorry.

magnetophon commented 4 years ago

After a long hiatus from my side, the summing issue has been resolved upstream.

Now the three summing methods are mostly in agreement. When I run the following with faust 2.20.2

import("stdfaust.lib");
bart = library("slidingReduce.lib");

signal = no.noise;
maxn = pow(2,13);
n = 3 ;
bruteForceSum(n,x) = x:seq(i, n-1, +(x@(i+1)));
slidingSum(n) = _ <: fi.integrator, fi.integrator@int(max(0,n)) : -;
process = sigcomp;
sigcomp =
  ( (signal:bruteForceSum(n) ) - ( signal:slidingSum(n) ) )
, ( (signal:bruteForceSum(n) ) - ( signal:bart.slidingSumN(n,maxn) ) )
, ( (signal:slidingSum(n)    ) - ( signal:bart.slidingSumN(n,maxn) ) )
;
sigs =
  signal <:
  (
    _
   ,bruteForceSum(n)
   , slidingSum(n)
   ,bart.slidingSumN(n,maxn)
  );

I get:

faustout = [ ...
 0 0 0; ...
 0 0 0; ...
 -1.1920929e-07 -1.1920929e-07 0; ...
 -9.26665962e-08 0 1.1920929e-07; ...
 -2.98023224e-08 0 0; ...
 1.1920929e-07 0 -1.1920929e-07; ...
 0 -7.4505806e-09 -5.96046448e-08; ...
 0 0 -5.96046448e-08; ...
 -1.1920929e-07 -5.96046448e-08 5.96046448e-08; ...
 -1.1920929e-07 1.1920929e-07 2.38418579e-07; ...
 -1.1920929e-07 0 1.1920929e-07; ...
 0 0 8.94069672e-08; ...
 0 0 -5.96046448e-08; ...
 2.38418579e-07 0 -1.78813934e-07; ...
 2.38418579e-07 0 -2.38418579e-07; ...
 2.38418579e-07 1.1920929e-07 -1.1920929e-07; ...
];

Interestingly, the error is almost always 1.1920929e-07 or double that or negative one of those. That also holds true for huge n's.

I guess those who actually understand floating point math are not surprised. I sure was!

When I compile with faust 2.5.23 I get;

faustout = [ ...
 0 0 0; ...
 0 0 0; ...
 2 0 -2; ...
 2 0 -2; ...
 -2.98023e-08 0 0; ...
 1.19209e-07 0 -5.96046e-08; ...
 1.19209e-07 0 -5.96046e-08; ...
 0 0 -5.96046e-08; ...
 0 0 5.96046e-08; ...
 -1.19209e-07 0 1.19209e-07; ...
 -2.38419e-07 0 1.78814e-07; ...
 -2.38419e-07 0 1.19209e-07; ...
 0 0 -5.96046e-08; ...
 2.38419e-07 0 -1.49012e-07; ...
 2.38419e-07 0 -2.38419e-07; ...
 -2 0 2; ...
];

If anyone has an idea about the cause, I'd be very interested. May I suggest adding a test so this error doesn't return?