cortex-js / compute-engine

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

Crash when serializing division with LaTeX #40

Closed bengolds closed 2 years ago

bengolds commented 2 years ago

Version: 0.4.4

Steps:

  1. Initialize Compute Engine
  2. Set jsonSerializationOptions.metadata = ['latex']
  3. Parse 1/x
  4. Try to print the MathJson for the resulting BoxedExpression.

Here's a quick node session replicating the problem:

> const ComputeEngine = require("@cortex-js/compute-engine").ComputeEngine
undefined
> let myCe = new ComputeEngine()
undefined
> myCe.parse("1/x").json
[ 'Divide', 1, 'x' ]
> myCe.jsonSerializationOptions.metadata = ['latex']
[ 'latex' ]
> myCe.parse("1/x").json
Uncaught TypeError: Converting circular structure to JSON
    --> starting at object with constructor 'global'
    --- property 'global' closes the circle
    at JSON.stringify (<anonymous>)
    at /Users/bengold/brilliant/node_modules/@cortex-js/compute-engine/dist/compute-engine.min.js:2:69692
    at Ne.serialize (/Users/bengold/brilliant/node_modules/@cortex-js/compute-engine/dist/compute-engine.min.js:2:69710)
    at ke.serialize (/Users/bengold/brilliant/node_modules/@cortex-js/compute-engine/dist/compute-engine.min.js:2:74791)
    at zt.serialize (/Users/bengold/brilliant/node_modules/@cortex-js/compute-engine/dist/compute-engine.min.js:2:267194)
    at vi (/Users/bengold/brilliant/node_modules/@cortex-js/compute-engine/dist/compute-engine.min.js:2:140335)
    at Oi.get json [as json] (/Users/bengold/brilliant/node_modules/@cortex-js/compute-engine/dist/compute-engine.min.js:2:163316)
arnog commented 2 years ago

Good find.

The problem occurred when requiring LaTeX metadata in a MathJSON output for a number, i.e. in the example above, it's actually "1" that causes an error to be thrown...

Fixed in 64e15037ddff7a3dbf2e0854729882aaa10bcb2e

bengolds commented 2 years ago

Ahhh, that's a funny one! Fantastic, thanks.

Relatedly, we did run into a situation where we wanted the latex to be preserved for numbers -- if the user types in 1e3, we'd like to know both that they typed 1e3 AND that it's equal to 1000. Similarly for 000.113!

arnog commented 2 years ago

Yeah, that makes sense.

Right now, I think the latex output is omitted only if the serialization of the numeric value is identical to the generated (or input) LaTeX, but perhaps it would be simpler to just always include the LaTeX when it is requested (i.e. via the metadata prop), even if it was redundant with the numeric serialization...

bengolds commented 2 years ago

The consistency seems totally fine to me.

When you get a chance, can you cut a release with 64e1503? We'd love to install directly to that hash, but our version of node is too low to do the rebuilding required :(

arnog commented 2 years ago

I've published compute-engine@0.5.0 that incorporate these changes and a corresponding mathlive@0.5.0.

bengolds commented 2 years ago

Thanks a million, Arno! Super excited to get these fixes (and some of the new features in compute engine) integrated :D