monome / teletype

monome eurorack module
GNU General Public License v2.0
195 stars 82 forks source link

JI OP Broken (Just Intonation) #129

Closed cmcavoy closed 6 years ago

cmcavoy commented 6 years ago

Please describe the bug.

JI OP is broken according to post on lines. Tracking it here.

I haven't confirmed this bug.

cmcavoy commented 6 years ago

Confirmed that this is still an issue. I asked for more information from @galapagoose, he was kind enough to send some more background,

I think I had some pseudo-code lying around that should fix it - only issue is it needed a log() lookup which seemed like it would have to be a custom LUT just for this op. I've resisted digging into the TT codebase so I'm not exactly sure how it would best be implemented.

If you're wanting to pursue it / post the info, the requirement specs are: Input is 2 integers The output is a number in the range 0 to V 1

Here's some code w/ bits of pseudo-code slotted in.

static void op_JI() {
    uint32_t n = pop();
    uint32_t d = pop();

    // normalize fraction between 1(root) and 2(octave)
    while( n < d ){
        n <<= 1;
    }
    while( n >= (d << 1) ){
        d <<= 1;
    }

    uint32_t ji = ( n << 8 ) / d; // 24bit LUT base

    // split `ji` into LUT index & fractional coefficient for xfade
    uint8_t ix = ji >> 16;
    uint8_t c  = (uint8_t)(( ji >> 8 ) & (uint32_t)0xFF );

    // lookup
    uint32_t a = ji_lut[ix];
    uint32_t b = ji_lut[ix+1]; // needs edge-check

    // xfade (integer math fun...)
    ji = a + c*(b-a)

    push( ji >> 4 ); // shift right to undo x16 range expansion in table
}

And this is the setup details, though this should likely be done offline and stored in flash rather than at startup.

#define LUT_SIZE 256

// create the LUT (pre-compute & save in flash)
uint16_t ji_lut[LUT_SIZE];
for( uint8_t i=0; i<LUT_SIZE; i++ ){
    // calculate as float / double then cast to u32
    ji_lut[i] = (uint32_t)( (5441.318221
                                * log_f(0.12231220585 * (float)(i+LUT_SIZE) / (float)LUT_SIZE)
                              + 4965.36721229)
                            * 16.0 // expand dynamic range *16 (still fits in s16)
                          );
}
trentgill commented 6 years ago

Passing these 3 tests should prove it: JI 4 1 -> 0 JI 1 4 -> 0 JI 3 2 -> 958

cmcavoy commented 6 years ago

I was able to write this in Python pretty easily based on the formula for computing cents from just intonation provided here. The original formula converted JI to cents (range of 1200 per octave), changing that to 1638 (the TT range for a single volt) works fine.

from math import log
def get_ji(n, d):
    ji = log(n/d) * 1638 / log(2)
    return int(ji)

It works for get_ji(3, 2) but changes the behavior of get_ji(4, 1) returning the fundamental pitch plus four octaves. According to the site above is the correct behavior, except the requirements above say The output is a number in the range 0 to V 1. Not sure what the history of that requirement is, or if it's still valid.

I'd like to try to fix this bug (I'm personally interested in playing around with just intonation based on this really great lines thread). I get basic C syntax, and this seems relatively easy - unless we don't have access to the math library and thus no access to log. My guess is a lot of the example code above is in place to get around this limitation.

Before I go down the path of working around not having log I wanted to confirm - can we use math? /cc @burnsauce @samdoshi @scanner-darkly @tehn

cmcavoy commented 6 years ago

The problem with the formula from here is that it will return negative values. get_ji(1, 4) returns -3276, which is correct - except we can't use that. So will need to return 0.

trentgill commented 6 years ago

tldr: i don't believe you can use log() on a microcontroller without an FPU (or it will be super slow).

//

My intention for the JI op was that it used 'normalized' JI ratios. In my readings that seems to be the traditional way to represent pitches – otherwise people start writing silly, unreadable fractions when they want to go up or down octaves. For this op, I anticipated the octave choice would be done separately, though perhaps integrating it as another parameter would make for terser scripts.

Regarding the python code you post, the problem is that it requires floating point math which we don't have on the teletype hardware. I believe you can still use , and the log() fn but owing to the nature of logarithms, you will end up truncating the result, unless you can get some kind of floating point result.

My solution above is to create a table of values that are the answer to your get_ji() function, to which we feed the result of the n/d division. Then I propose a linear interpolation between the nearest two points in the table to improve tuning accuracy & decrease LUT size.

This said, the equation you found is probably a better candidate to create the LUT! My solution was working backward from the frequency-to-midi converter in puredata...

cmcavoy commented 6 years ago

Oh wow, yeah - I really don't understand the TT hardware! Thanks for the explanation @trentgill. I knew the TT language didn't support floating point numbers, but didn't realize the actual CPU didn't either. Will really need to change my thinking if I want to try tackle bugs!

Also, understood on the normalization to a single octave. That makes sense (cents? #dadjoke).

trentgill commented 6 years ago

Ahoy. Just a little headsup I found an entirely alternate solution that uses prime factorization & addition/subtraction to do this in a cpu & memory optimized way. Sorry for all the blather about lookup tables.

Hoping to have a PR by the weekend.

cmcavoy commented 6 years ago

Sounds very interesting @trentgill! Looking forward to learning from the PR. Given the state of 2.2, there might be a bit of juggling, but I'm definitely interested in using the JI op!

As an aside, I asked a couple of related questions in the Modern C thread on lines. It led to some interesting suggestions. There's a log function that's available through libfixmath that doesn't use floating point types. I'm a total C newb, but started messing around with it. I'm actually stuck on compiling some test code - won't get into the details here, I just need to sit down for a while and figure it out. Anyway, if the log solution w/ libfixmath is appealing - there's good info in that lines thread. Also - the idea of using a LUT was definitely 👍 by the folks in that thread, so...still a viable solution!

cmcavoy commented 6 years ago

Looking forward to playing around with this this weekend @trentgill thanks! I'm going to close this issue, but I'm interested in the solution you found...do you have a link you can share? I've looked at the code, but would be interested in discussion about why it works if that's easy to find. Thanks!

trentgill commented 6 years ago

@cmcavoy Finally got around to thinking about this for more than a few seconds.

The solution uses the first two log laws:

log A + log B = log AB

and

log A − log B = log A / B

Thus when we go back to your initial equation:

ji = log(n/d) * 1638 / log(2)

consider the 1638 / log(2) as a simple constant 'k', then all we’re left with is log(n/d).

With the second law this becomes:

ji = k ( log(n) - log(d) )

Now the fun part which uses some assumptions about JI. We know that ’n’ and ‘d’ will be numbers made up of prime-factors less than 13 (see the first paragraph of this http://xenharmonic.wikispaces.com/13-limit http://xenharmonic.wikispaces.com/13-limit), thus we only need to store the computed values of log(x) where x is all the primes up to 13 (not many!).

An example! 40 / 39. 40 = 2225, and 39 = 313, thus our equation becomes:

ji = k ( log(2) + log(2) + log(2) + log(5) - ( log(3) + log(13) ) )

That’s the trick! All we do is save the values of log(2),(3) etc, and pull them from an array with some addition & subtraction. The most difficult intensive part in the process is the prime-factorization. I tried to make that short-circuit as much as possible, plus used a divide & multiple, rather than divide & modulo (though i’m not sure the compiled code is any different, nor whether there is hardware divide on avr32?). nb: the values stored in the array have the 1638/log(2) constant applied to them already.

Oh and one more thing to note: when n/d is 2/1, if you expand the equation, log(n/d) = log(2) thus cancelling the log(2) at the end. Which is obvious true as the JI ratio 2/1 is 1 octave (or 1638*1)!

On Nov 17, 2017, at 6:16 PM, Chris McAvoy notifications@github.com wrote:

Looking forward to playing around with this this weekend @trentgill https://github.com/trentgill thanks! I'm going to close this issue, but I'm interested in the solution you found...do you have a link you can share? I've looked at the code, but would be interested in discussion about why it works if that's easy to find. Thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/monome/teletype/issues/129#issuecomment-345391473, or mute the thread https://github.com/notifications/unsubscribe-auth/AAD4_fTXnwArI7zyGuzfmLekGmETyGsYks5s3hPqgaJpZM4QTzSs.

cmcavoy commented 6 years ago

Wow @trentgill this is super helpful, thanks for the write up! And the fix. Really interested in just intonation, very thankful for this tool to help experimentation.