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

eval unable to perform calculations on "Unit-less" numbers #651

Closed
ellis opened this Issue Apr 19, 2016 · 5 comments

Comments

Projects
None yet
3 participants
@ellis

ellis commented Apr 19, 2016

This expression math.eval("(100m / 10m) + 1") results in the following error:

TypeError: Unexpected type of argument in function add (expected: Array or Matrix, actual: number, index: 1)

The reason is that 100m / 10m is still a Unit rather than a numeric value.

My scenario for needing this is when evaluating a sequence of expressions something like this:

totalSize = 100m
stepSize = 10m
steps = totalSize / stepSize + 1

(Btw, being able to evaluate math expression in the browser using math.js is really impressive and a ton of fun!)

@ericman314

This comment has been minimized.

Collaborator

ericman314 commented Apr 19, 2016

This is definitely a bug we should fix.

@josdejong, any preference on this?

Option 1: We can have Units.js strip the units off of any result that comes back dimensionless (unless predictable: true), so that (100m) / (10m) would return the number 10, not the unit 10.

Option 2: We can add implementations for add(number, Unit) and add(Unit, number) that fail if the Unit has any units. The new functions would return a number. I like this because it avoids the dependence on a config property.

@ellis, because of the precedence of implicit multiplication, math.js parses the expression "(100m / 10m) + 1" as: "(((100 m) / 10) m) + 1", giving 10 m^2 + 1 which will result in an error. You can avoid this by putting each unit in parentheses.

math.parse("((100 m) / (10 m)) + 1").toString({parenthesis: 'all'});

  '((100 m) / (10 m)) + 1'
@ellis

This comment has been minimized.

ellis commented Apr 19, 2016

@ericman314 Thanks for the clarification. The case where I originally encountered the error was using variables, like shown in the example with totalSize and stepSize -- I hadn't realized that (100m / 100m) + 1 was actually causing a different error. math.parse is useful to know about now.

Regarding Option 2, a pragmatic issue might be that it would need to be applied to all kinds of functions. For example, in my real use-case, I need an integer number of steps required to cover a total distance, using the ceil function:

totalSize = 100m
stepSize = 12m
steps = ceil(totalSize / stepSize) # should equal 9
@josdejong

This comment has been minimized.

Owner

josdejong commented Apr 19, 2016

@ericman314 I like both options. I think @ellis has a good point though that having a dimensionless unit automatically be changed into a number (when {predictable: false}) can be more convenient from a users point of view. It will be more work to implement I think but it could be worth it.

@ericman314

This comment has been minimized.

Collaborator

ericman314 commented Apr 19, 2016

Resolved with PR #653.

Requires config option {predictable: false}, which is the default.

var totalSize = math.unit('100m');
var stepSize = math.unit('12m');
console.log(math.ceil(math.divide(totalSize, stepSize)));
console.log(math.eval('ceil(100m / (12m))'));
console.log(math.eval('100m / (12m) + 1'));

Output:

9
9
9.333333333333334

@ericman314 ericman314 closed this Apr 19, 2016

@ellis

This comment has been minimized.

ellis commented Apr 20, 2016

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment