cortex-js / compute-engine

An engine for symbolic manipulation and numeric evaluation of math formulas expressed with MathJSON
https://cortexjs.io
MIT License
356 stars 42 forks source link

new parse error when handling InvisibleOperator #146

Closed dcbradley closed 6 months ago

dcbradley commented 6 months ago

Description

Recent commit 60002e6 for #143 causes new parse errors that are probably not intended.

Steps to Reproduce

const ce = new ComputeEngine();
var expr = ce.parse("x(x+1)");
console.log(JSON.stringify(expr.json));

Actual Behavior

["x",["Add",["Error",["ErrorCode","'incompatible-domain'","Numbers",["FunctionOf",["VarArg","Anything"],"Anything"]],"x"],1]]

Expected Behavior

["Multiply","x",["Add","x",1]]

Environment

Observed in Debian Linux 11 with node-v20.10.0-linux-x64.

Additional Debugging Information

The expected behavior is observed in the commit prior to 60002e6 (519e577). When parsing with canonical=false, there is no parse error, and the following is the result:

["InvisibleOperator","x",["Delimiter",["Add","x",1]]]

Sorry for causing trouble!

dcbradley commented 6 months ago

Sorry ... I changed my test case at the last minute before submitting the bug report, so there is an inaccuracy in the behavior I described. Here's the code that worked without error prior to 60002e6:

const ce = new ComputeEngine();
var expr = ce.parse("x(x+1)",{canonical: ["InvisibleOperator"]});
console.log(JSON.stringify(expr.json));

When canonical=true, the the parser produces the error both before and after the commit, so the new behavior is more consistent, but it's still not clear to me if it is intended.

dcbradley commented 6 months ago

The origin of the parse error is commit 7bc1abb. Prior to that, the following code did not produce a parse error:

const ce = new ComputeEngine();
var expr = ce.parse("x(x+1)");                                                                                                                      
console.log(JSON.stringify(expr.json));

The result was:

["Multiply","x",["Add","x",1]]

After 7bc1abb, the result is

["x",["Add",["Error",["ErrorCode","'incompatible-domain'","Numbers",["FunctionOf",["VarArg","Anything"],"Anything"]],"x"],1]]

So 0.20.0 has the problem when parsing with default canonicalization and 0.19.1 doesn't.

arnog commented 6 months ago

Hmmm... That's interesting.

So, when the symbol "x" is first encountered, because it is an undeclared symbol, its domain is inferred. The expression looks like a function call, so the domain of "x" inferred to be a function. When it is later used as a variable, this causes the domain error. I could try to backtrack the domain inference in case of an error.

BTW, the reason why this worked before is because there were cases where sub-expressions were not correctly validated.