complex-js icon indicating copy to clipboard operation
complex-js copied to clipboard

Bad multiply-division parsing

Open sytabaresa opened this issue 5 years ago • 2 comments

Hi, nice library. lightweight and so useful.

But trying to make a app, I note that some expressions are not parsed correctly:

Complex.compile('1/a*b*c',['a','b','c'])

returns a optimized function in the browser:

ƒ: anonymous(Complex,a,b,c) {
return this[0].divide(a.multiply(b)).multiply(c)
}

and

Complex.compile('(1/a)*b*c',['a','b','c'])

returns

ƒ: anonymous(Complex,a,b,c) {
return this[0].divide(a).multiply(b).multiply(c)
}

that are different expressions.

* and / operators should be the same precedence and the correct result is the last (a chain of operators without grouping).

Thanks

complex-js version: 5.0.0

sytabaresa avatar Dec 27 '19 17:12 sytabaresa

@sytabaresa thanks for bringing my attention to this, I'll see if I can implement a pull request for this.

In the meantime, I don't know if you're looking for a work-around but I just tested a gist I wrote a while back, and the problem does not seem to exist there so I'll share it here in case it helps you.

To get your function, you can compile the parser.pegjs file using https://pegjs.org/online, then after downloading the gist as a zip file, edit demo.js to look like this:

// import compiler defined in compiler.js
const { compile } = require('./compiler')
// use this mapping for named parameters
const map = (a, b, c) => ({ a, b, c })
// compile these expressions
const f = compile('1/a*b*c', map)
const g = compile('(1/a)*b*c', map)

console.log(f(2, 3, 4))
console.log(g(2, 3, 4))

They both output 6.

P.S. If you want the functions to accept instances of the complex class instead, you'll need to modify the operators in expressions.js:

const Complex = require('complex-js');

// initialize operator expressions
const [
  add, subtract,
  multiply, divide,
  mod, power,
  negate, invoke
] = [
  (a, b) => a.add(b), (a, b) => a.subtract(b),
  (a, b) => a.multiply(b), (a, b) => a.divide(b),
  (a, b) => a.modulo(b), (a, b) => a.power(b),
  (a) => Complex.negate(a), (fn, ...a) => fn(...a)
].map(operation)

I'll update this issue once I implement the fix.

patrickroberts avatar Dec 27 '19 21:12 patrickroberts

@sytabaresa Hey, I've got a new version of the library in progress, but so far things look pretty promising! This order of operations bug has been fixed in the new compiler which uses nearley for parsing and moo for lexing, and the updated call signature looks like this:

Complex.compile('1/a*b*c', (a, b, c) => ({ a, b, c }))

Since the project isn't on npm yet, you'll need to git clone the repository and use npm run build to generate the dst directory with the bundled output, but when I get around to publishing everything on npm, the code will be pre-bundled, and none of the devDependencies will be required to use the library.

Let me know what you think and if this is sufficient for your purposes.

Edit

It's on npm now so you should just be able to run:

npm i complex-js moo nearley

patrickroberts avatar Feb 27 '20 15:02 patrickroberts