ben-hayes / neural-waveshaping-synthesis

efficient neural audio synthesis in the waveform domain
Mozilla Public License 2.0
185 stars 14 forks source link

Off-by-one error in index computation in FastNEWT #10

Open vvolhejn opened 2 years ago

vvolhejn commented 2 years ago

Hi, in FastNEWT, the continuous array index is computed as (code):

idx = self.table_size * (x - self.table_min) / (self.table_max - self.table_min)

I believe the correct version would be

idx = (self.table_size - 1) * (x - self.table_min) / (self.table_max - self.table_min)

For instance, imagine table_min=0, table_max=1, table_size=2. Then if x == 0.5, we would get idx == 1.0 in the current version, whereas it should be idx == 0.5 (in general, idx == x).

Edit: Additionally, I think the behavior on the left and right sides of the lookup table is different. On the left, there is linear extrapolation, on the right, a constant. I'm running my reimplementation rather than the original code but the computation should be the same.

Screenshot 2022-04-04 at 15 59 18

The reason is that lower is first clipped and then upper is computed as clip(lower + 1). This means that for values lower than min_value, we have lower == 0 and upper == 1 whereas for values larger than max_value, we have lower == upper == table_size - 1.