Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bad multiply-division parsing #6

Open
sytabaresa opened this issue Dec 27, 2019 · 2 comments
Open

Bad multiply-division parsing #6

sytabaresa opened this issue Dec 27, 2019 · 2 comments
Assignees
Labels
bug Unintended behavior resulting from a logical error

Comments

@sytabaresa
Copy link

sytabaresa commented Dec 27, 2019

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 sytabaresa changed the title Bad multiply-divition parsing Bad multiply-division parsing Dec 27, 2019
@patrickroberts
Copy link
Owner

patrickroberts commented Dec 27, 2019

@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 patrickroberts self-assigned this Dec 29, 2019
@patrickroberts patrickroberts added the bug Unintended behavior resulting from a logical error label Dec 29, 2019
@patrickroberts
Copy link
Owner

patrickroberts commented Feb 27, 2020

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unintended behavior resulting from a logical error
Projects
None yet
Development

No branches or pull requests

2 participants