From 7e7ea4ebc5f99b5d757b199c4cbe2b47ec27d443 Mon Sep 17 00:00:00 2001 From: Lumi Pakkanen Date: Thu, 8 Feb 2024 02:57:38 +0200 Subject: [PATCH] Pre-reduce fraction multiplication and division to stay within safe limits --- src/__tests__/fraction.spec.ts | 7 +++++++ src/__tests__/monzo.spec.ts | 6 ++++++ src/fraction.ts | 16 ++++++++++++++-- 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/__tests__/fraction.spec.ts b/src/__tests__/fraction.spec.ts index 16c12e1..9ee9f94 100644 --- a/src/__tests__/fraction.spec.ts +++ b/src/__tests__/fraction.spec.ts @@ -440,4 +440,11 @@ describe('Fraction', () => { foo = foo.mul(foo); } }); + + it('multiplies large cancelling factors', () => { + const one = new Fraction('1234567890/987654321').mul( + '987654321/1234567890' + ); + expect(one.equals(1)).toBe(true); + }); }); diff --git a/src/__tests__/monzo.spec.ts b/src/__tests__/monzo.spec.ts index ebe3246..c54434b 100644 --- a/src/__tests__/monzo.spec.ts +++ b/src/__tests__/monzo.spec.ts @@ -89,6 +89,12 @@ describe('Fraction to monzo converter', () => { ).toBeTruthy(); }); + it('converts a Pythagorean interval', () => { + const [monzo, residual] = toMonzoAndResidual('129140163/134217728', 6); + expect(monzo).toEqual([-27, 17, 0, 0, 0, 0]); + expect(residual.equals(1)).toBe(true); + }); + it('leaves residual 0 for zero (vector part)', () => { const [monzo, residual] = toMonzoAndResidual(0, 1); expect(residual.equals(0)).toBeTruthy(); diff --git a/src/fraction.ts b/src/fraction.ts index 4eb74bd..f4f6415 100644 --- a/src/fraction.ts +++ b/src/fraction.ts @@ -528,7 +528,13 @@ export class Fraction { **/ mul(other: FractionValue) { const {s, n, d} = new Fraction(other); - return new Fraction(this.s * this.n * s * n, this.d * d); + // Must pre-reduce to avoid blowing the limits + const ndFactor = gcd(this.n, d); + const dnFactor = gcd(this.d, n); + return new Fraction( + this.s * (this.n / ndFactor) * s * (n / dnFactor), + (this.d / dnFactor) * (d / ndFactor) + ); } /** @@ -541,7 +547,13 @@ export class Fraction { if (n === 0) { throw new Error('Division by Zero'); } - return new Fraction(this.s * this.n * s * d, this.d * n); + // Must pre-reduce to avoid blowing the limits + const nFactor = gcd(this.n, n); + const dFactor = gcd(this.d, d); + return new Fraction( + this.s * (this.n / nFactor) * s * (d / dFactor), + (this.d / dFactor) * (n / nFactor) + ); } /**