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

Algebraic differentiation #411

Closed
wants to merge 6 commits into from

Conversation

BigFav
Copy link
Collaborator

@BigFav BigFav commented Jul 21, 2015

#35

BigFav added 5 commits May 31, 2015 02:49
…te, with a few functions as well. Still does not handle argument errors well, even though it returns the same it received.
… a variable base log, and added a test case. Adjusted the chain rule to be after the switch statement (saves ~1640 bytes and is more modular).
@BigFav BigFav mentioned this pull request Jul 21, 2015
@josdejong
Copy link
Owner

This is really awesome! As far as I tested it all works like a charm!

A few thoughts:

  • I'm not sure if it's a good idea to release it now already, it maybe better to release in conjunction with a function simplify: for example derivative(2*x^3, x) returns 2 * 3 * 1 * x ^ (3 - 1), which is correct, but I think people would expect to get 6 * x ^ 2 returned. Maybe derivative should automatically call simplify at the end.
  • You mentioned this before, but to keep the unit tests readable/maintainable it may be better to replace the huge nested trees with Nodes (like here) with a short math.parse('...').

What do you think?

@BigFav
Copy link
Collaborator Author

BigFav commented Jul 23, 2015

I'm not sure if it's a good idea to release it now already, it maybe better to release in conjunction with a function simplify:

This is one thing I wanted to ask about, but wasn't sure if it was considered directly related or simply an exhibit of when simplify would be particularly convenient. This distinction is important because depending on which case we consider this to be, there may or may not be a threshold for what is "simplified enough" for this to be released.

for example derivative(2*x^3, x) returns 2 * 3 * 1 * x ^ (3 - 1), which is correct, but I think people would expect to get 6 * x ^ 2 returned.

Agreed, most other interfaces return (at least partially) simplified equations.

Maybe derivative should automatically call simplify at the end.

If we have a simplify function, all algebraic functions should call it at the end lol. I can give that a shot as well, but I will also create an issue (#412) for that to see if anyone else is interested in tackling this problem.

You mentioned this before, but to keep the unit tests readable/maintainable it may be better to replace the huge nested trees with Nodes (like here) with a short math.parse('...').

In some places it isn't possible, given a lack of ParenthesisNodes to enforce an order of operations, but an implied order given the nesting of the Nodes, but in a lot of places it is possible. I can go back and try to clean that up, and I think that would be for the best.

@josdejong
Copy link
Owner

sounds good, thanks!

…tions are possible. Used config.number on ConstantNodes that I missed on my original commits.
@BigFav
Copy link
Collaborator Author

BigFav commented Jul 25, 2015

I have no idea where those errors are coming from. They seem unrelated to derivative (the errors are in eval), appear in places this commit didn't change (e.g. derivative of a constant), and don't occur on my machine.

@josdejong
Copy link
Owner

hm, I will have a look at it, see what it does on my machine.

@BigFav
Copy link
Collaborator Author

BigFav commented Jul 29, 2015

So I pulled from master, and it shows the errors. I think the error is that after the updates, the expression parser will not accept any SymbolNodes that are not in scope. Is it possible to add a variable to the scope without giving it a value?

@josdejong
Copy link
Owner

@BigFav I found the issue, it's not your fault... In order to implement lazy loading for the functions I had to change the way function transforms work. You can't used them with lazy loaded functions anymore. I see the docs do not reflect this and there was no error message too. Sorry for the trouble.

I've now added an error message in case there is a transform attached to a factory function. And to solve the issue with derivative, I have moved the transform part of derivative.js to a separate file derivative.transform.js, which directly registers itself to the path math.expression.transform.

I've merged your work in a separate branch for now: https://github.com/josdejong/mathjs/tree/algebraic_differentiation. Though not yet ready to go public (related functions simplify and integrate are still missing), I think it will be really cool to merge this into develop as a undocumented, experimental feature so we can start playing around with it. This stuff is just too much fun to wait any longer :D.

@BigFav BigFav closed this Oct 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants