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

Parser: simplify an expression to another expression just with undefined variables. #907

Closed
paulobuchsbaum opened this Issue Jul 28, 2017 · 28 comments

Comments

Projects
None yet
3 participants
@paulobuchsbaum
Contributor

paulobuchsbaum commented Jul 28, 2017

The mathjs package is very useful and powerful. However, there is a feature that would be awesome if it had: The ability to solve an expression with the variables that already exist, reducing it to a small and simplified expression with fewer variables, performing all operations with variables already defined. It would be like a more advanced simplification. # # #

@josdejong

This comment has been minimized.

Owner

josdejong commented Jul 29, 2017

Thanks for sharing your ideas! That would be interesting indeed.

@josdejong josdejong added the feature label Jul 29, 2017

@firepick1

This comment has been minimized.

Collaborator

firepick1 commented Jul 29, 2017

Paolo, please take a look at the discussion #811
I've implemented an expression memoizer that greatly improves the performance of derivative and eval.
That memoizer isn't yet part of mathjs mostly because "it doesn't feel right as is". However, I'd be curious on your take on the discussion and see if our two feature requests have some common ground. :D

@paulobuchsbaum

This comment has been minimized.

Contributor

paulobuchsbaum commented Jul 29, 2017

Thanks, firepick1. I will try it and after I give some feedback about it.
It is in firepick/equation.js
I need some kind of more powerful and generic simplification for my project.

@paulobuchsbaum

This comment has been minimized.

Contributor

paulobuchsbaum commented Jul 29, 2017

I've tried the fastSimplify() from equations. Indeed it's blazingly fast:

require("kinann");
var Equations = require("kinann/src/Equations");
 var eq = new Equations();
eq.fastSimplify( ......);

However I don't get to reproduce my simplification and replacement idea.

Using mathjs syntax, with a hypothetic function simplifyPlus:

var m = require('mathjs');
var p = new m.parser();
p.eval('x=5; y=10');
var expr = p.simplifyPlus('x^2 - 2*y*z  - 20 + 5*z')
console.log(expr.toString())    //  display  '25z + 5'

p.eval('b=2');
expr = p.simplifyPlus('( log(a,b) + log(a,c)  ) /  sin(b* c * pi/8) ');
console.log(expr.toString());    //  display  'log(a,2) + log(a,c) / sin(c * pi / 4)'
p.eval('c=2');
expr = p.simplifyPlus(expr);
console.log(expr.toString());    //  display  '2log(a,2)' 

p.set('myfunc', function (a,b,c) 
  {  if (arguments.length !== 3) return NaN;
  	 return a+b*c;} );  

expr = p.simplifyPlus('myfunc(t,u,5)');
console.log(expr.toString());    //  display  'myfunc(t,u,5)'
p.eval('t=3'); 
expr = p.simplifyPlus(expr);
console.log(expr.toString());    //  display  'myfunc(3,u,5)'
p.eval('u=4'); 
expr = p.simplifyPlus(expr);
console.log(expr.toString());    //  display  '23'

@josdejong

This comment has been minimized.

Owner

josdejong commented Jul 30, 2017

@paulobuchsbaum thanks for sharing these desired usage examples, that helps getting more clear what we want to be able to solve.

Basically it's resolving variable values. This shouldn't be too hard: the simplifier is already resolves constants like simplify("2 + 2") into 4, so resolving simplify("a + 2", {a : 2}) into 4 should be doable.

@paulobuchsbaum

This comment has been minimized.

Contributor

paulobuchsbaum commented Jul 30, 2017

Thanks for your answer. Exactly! It could be implement like a functionality extension of simplify method. It does not seem to be rocket science for those who participated in the development of the mathjs project, so useful to the community.

@firepick1

This comment has been minimized.

Collaborator

firepick1 commented Jul 30, 2017

Perhaps we just need a resolve primitive that resolves an expression tree with an object such as
var defs= {x:5,y:10};
Then we would just compose simplify(resolve(expr, defs)). Does that suit?

@paulobuchsbaum

This comment has been minimized.

Contributor

paulobuchsbaum commented Jul 30, 2017

Such an approach would also resolve the issue raised. I believe that from the standpoint of consistency this would be the best approach because it separates resolution from simplification, but the approach of incorporating simplification into the evolving state of the environment is attractive from the point of view of practicality.

It would be great to see the best math library that exists for Javascript even better.

@josdejong

This comment has been minimized.

Owner

josdejong commented Jul 30, 2017

Then we would just compose simplify(resolve(expr, defs)). Does that suit?

indeed, though I think in practice you would always use resolve in combination with simplify, and not use resolve on its own. So it maybe more practical to (at least for the public API) implement an optional scope with variables that you can pass to simplify, like:

simplify(expr)
simplify(expr, scope)
@josdejong

This comment has been minimized.

Owner

josdejong commented Jul 30, 2017

@firepick1 would you be interested to see if you can implement this feature?

@firepick1

This comment has been minimized.

Collaborator

firepick1 commented Jul 30, 2017

@josdejong

This comment has been minimized.

Owner

josdejong commented Jul 31, 2017

cool, thanks!

@firepick1

This comment has been minimized.

Collaborator

firepick1 commented Aug 1, 2017

Turns out simplify already has an optional second arg, so we are looking at:
simplify(expr, rules, scope);
This might be (?) a bit awkward in that users will frequently have to do this:
simplify(expr, null, scope);
I propose allowing the following via type checking:
simplify(expr, scope);
Any objections?

@josdejong

This comment has been minimized.

Owner

josdejong commented Aug 1, 2017

heee, really?! that's cool! This hidden feature is missing in the docs.

I think that scope will be more commonly used than rules, so changing the order will be a good idea. This will be a breaking change though, so we need to schedule it for v4. Maybe we can keep it compatible by checking whether the second argument is an Array (rules) or object (scope).

@firepick1

This comment has been minimized.

Collaborator

firepick1 commented Aug 1, 2017

I just discovered that mathjs supports typed argument parsing, so it is indeed possible to support both:

simplify(expr, rules, scope);
simplify(expr, scope);

Therefore, this need not be a breaking change.

Oh. Here are the proposed tests for resolve. @jos, @paulobuchsbaum, (and community) please review for expected behavior:
test.txt

@paulobuchsbaum

This comment has been minimized.

Contributor

paulobuchsbaum commented Aug 2, 2017

I've saw the test.txt. It's perfect.
I will add one additional example with undetermined value inside a function.
It's recursive.

simplifyAndCompare('combinations( ceil (abs (sin(x)) *y) ), abs(x) )', 'combinations(ceil(0.909297 y ), 2)', {x:-2});

@firepick1

This comment has been minimized.

Collaborator

firepick1 commented Aug 2, 2017

That didn't work, but this did:
simplifyAndCompare('combinations( ceil(abs(sin(x)) * y), abs(x) )', 'combinations(ceil(0.9092974268256817 * y ), 2)', {x:-2});
Acceptable?

@paulobuchsbaum

This comment has been minimized.

Contributor

paulobuchsbaum commented Aug 2, 2017

Of course!!!

firepick1 added a commit that referenced this issue Aug 2, 2017

@firepick1

This comment has been minimized.

Collaborator

firepick1 commented Aug 2, 2017

Perfect!
@jos simplify2 branch is ready for merging into master. All existing tests pass and changes are therefore compatible with existing behavior. New tests added. Changes comprise fix for #907 as well as performance improvements for simplify via simplify.simplifyCore(). Please advise as to next steps.
@paulobuchsbaum you can directly use simplify2 if need is urgent or wait for master merge.

@paulobuchsbaum

This comment has been minimized.

Contributor

paulobuchsbaum commented Aug 2, 2017

@firepick1, how long do you think this update will be available for "NPM update mathjs" in node.js?
I have no experience in github projects, so I'm a little lost.
As far as I know, the simplify function, which acts on either the expression or a root node, will officially have an additional parameter, the third, which will contain a scope, which is an object with as many properties as variables to be defined.
In any case, I'm very happy because I'll be able to use this in my project.

@firepick1

This comment has been minimized.

Collaborator

firepick1 commented Aug 2, 2017

@paulobuchsbaum timing on master merge for npm is up to Jos. It might be easier to wait for the merge. The simplify2 branch is publicly available directly on GitHub but I think the inconvenience of figuring out how to use that might not be worth your time. I'm also not sure what the mathjs process for documentation is just yet, so we'll need to wait for Jos to let us know how to proceed.

@paulobuchsbaum

This comment has been minimized.

Contributor

paulobuchsbaum commented Aug 2, 2017

@firepick1 and @josdejong, I have no words to thank all this. I believe this will be helpful to many people in the community.

@josdejong

This comment has been minimized.

Owner

josdejong commented Aug 2, 2017

Great, thanks @firepick1! Nice that we don't have breaking changes after all.

I can do a next release of math.js this weekend.

text.txt is indeed what we're looking for. I'm in doubt though about resolving a variable that is a string:

math.simplify('y', {x:2, y:"1+x"})

This is really powerful but a string is also "just" a valid value and might not always contain an expression that needs to be resolved, for example:

math.simplify('y', {x:2, y:"1+x", text: "hello world :)"}) // whoops...

Thinking aloud: how about an API where we separate expressions from scope (the latter only containing values, no expressions):

// math.simplify (expression: string)
// math.simplify (expressions: Object)
// math.simplify (expression: string, scope: Object)
// math.simplify (expressions: Object, scope: Object)
// ...and the variant with rules

// usage:
math.simplify('x + 3', { x: 2 });
math.simplify({ x: 2, y: '1+x', result: 'y' });
math.simplify({ y: '1+x', result: 'y + size(text)[0]' }, { x: 2, text: 'Hello :)' });

The common use case is not using texts, and this way the common use case is very easy (pass a single object with all your expressions and values), and in case you do need to pass special strings or you have all your values in a separate scope, you can easily pass this as the second argument.

@firepick1

This comment has been minimized.

Collaborator

firepick1 commented Aug 2, 2017

Good catch--I didn't know about string values in mathjs. 😮
Simple fix is to resolve scope Node values instead of string values (fix checked in):
simplify("x+y", {x:1, y: math.parse("x+x")}

On the extension of simplify to allow expression objects as a handy shortcut, this might be a duplication of functionality provided by other well-established existing libraries such as lodash: See http://underscorejs.org/#mapObject

@josdejong

This comment has been minimized.

Owner

josdejong commented Aug 5, 2017

I don't think passing a map as first argument is really duplicate of underscore functions, this is really about the handiest API for simplify to pass more than one expression. Passing it via scope conflicts with string values, so we need to have an other way.

Using simplify("x+y", {x:1, y: math.parse("x+x")} can be fine for now, we can always extend the function if there is a demand for it. Let's start simple :D

@firepick1

This comment has been minimized.

Collaborator

firepick1 commented Aug 5, 2017

Simple for now works for me. I am easily confused. :)

@josdejong

This comment has been minimized.

Owner

josdejong commented Aug 6, 2017

LOL

@josdejong

This comment has been minimized.

Owner

josdejong commented Aug 6, 2017

The extended simplify is now available in v3.16.0, and is way faster thanks to @firepick1 !

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