-
-
Notifications
You must be signed in to change notification settings - Fork 480
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
Implement diff
format symbolic derivative in new symbolics
#6756
Comments
This comment has been minimized.
This comment has been minimized.
Author: Golam Mortuza Hossain |
comment:2
Attachment: trac_6756-diff-derivative-sage.patch.gz I am interested in reviewing this, but I'm not sure that I can at this time. I have a heavily modified
When I work from the command line, I get things like
So there must be some calculus or symbolic patches that I'm missing. Any suggestions? |
comment:3
Replying to @ncalexan:
Thanks Nick for trying it out. From this error, it seems "diff_derivative_level" flag wasn't set to
This looks really weired to me. Does it work before calling "set_diff_derivative_level()"? I can suggest you to check three things (1) Ensure in "symbolic/pynac.pyx" you have a line:
May be you can set it to "1", to enable "diff" by default. (2) In "symbolic/all.py" you have a line:
(3) In "calculus/calculus.py" the following line is commented out or removed
If these three lines are fine then it should work. Some doctests may it still Please let me know if that works. Best, |
comment:4
Sorry, same problems with a new 4.1.1 binary. It's possible I'm building in the wrong order (patch, failed tests, then new spkg, touch patch, rebuild) but that's two systems not working. |
Attachment: trac_6756-referee-patch-ncalexan-1.patch.gz |
comment:5
The second patch makes this at least work for me, and fixes (maybe an ordering issue?) a test that doesn't pass on my box. I'm not done reviewing yet, but at least it works as advertised. Question: how do I pattern match on this? |
Reviewer: Nick Alexander |
comment:6
Aha, very interesting, you pattern match via the |
comment:7
I would like to evaluate various derivatives numerically, but there's no obvious way to do so.
I would like to have my So I need to match the wild cards, then calculate the function, then substitute. Just longer, that's all. |
comment:8
Hi Nick, As you suggested, I moved the "set_diff_derivative_level" into pynac library. |
Attachment: trac_6756-diff-derivative-sage-v2.patch.gz Apply only this patch and ignore first two patches |
comment:9
Patches updated to include Nick's suggestion to move "set_diff_derivative_level" into pynac library. |
comment:10
It seems that applying chain rule to these inert derivatives can lead to wrong answers:
Compare with the answer we get now:
This example is from p. 26 (second page) in
You can get the article here: http://doi.acm.org/10.1145/1089338.1089343 It was the first reference on the paper mentioned by RJF in this post: http://groups.google.com/group/sage-devel/msg/6db333cbf8ea0a53 The paper RJF cites is
which you can get here: http://doi.acm.org/10.1145/1089402.1089405 I suggest removing the chain rule from this implementation altogether and keeping it as an alternative inert derivative. Then the implementation can be moved within the Sage library completely as a subclass of SFunction. We should also implement conversions between the partial derivative format and this one. This page gives a recipe on how this can be done: (The link points to the google cache since mapleprimes.com is down.) Nick, if you give examples of how you want pattern matching on derivatives to work, we can see how to make the current implementation support these. Pattern matching capabilities of |
comment:11
Replying to @burcin:
Thanks for the reference and I am certainly aware of this issue. If you read the http://groups.google.com/group/sage-devel/browse_thread/thread/c8d257981e3e3d98 Following are my comments: (1) f(y,y) is not a genuine function of two variables. So asking for its chain rule, pretending it to be a function of multiple variables, is itself incorrect assertion. (2) I don't think even D[] derivative output is much better in this regard, specially if you implement and allow substitution of f(y,y). For example. consider the substitution: f(y,y) = y in "D0(y, y) + D1(y, y)", then you will get the same wrong answer from D[] as from "diff". (3) Applying chain rule is just an option in new diff implementation and certainly not the default option. However, this could be useful sometime (for example in computing Euler-Lagrange equation using functional derivative of a formal functional "S(f(y), g(y))" this feature is needed. In fact, this was the reason for implementing this feature.) So I am sorry to differ from your opinion about removing this feature.
As I posted in the sage-devel, I initially implemented this within Sage as SFunction Why do you want to have a slower implementation of diff than a faster one?
I agree that we need this conversion at least to restore compatibility of D[] with Maxima which is badly broken now because of D[] derivative. However, I don't agree that I should implement this conversion and certainly not as a pre-requirement for accepting this patch. Given above arguments, I am reverting back to "needs review" status. |
comment:12
Replying to @golam-m-hossain:
This substitution doesn't make sense mathematically. D0(y,y) doesn't contain f(y,y), there is nothing to substitute. Bill Page had answered this point earlier: http://groups.google.com/group/sage-devel/msg/e6ded8f5e28a5aab This message by him in the same thread might be more helpful: http://groups.google.com/group/sage-devel/msg/98cc070640578f0c Note that with these patches, the result of |
comment:13
Replying to @burcin:
Hmm, if you bring mathematical sense into argument then does it
This is not accurate. Above will happen only when an user wants to apply chain rule by explicitly setting |
comment:14
Replying to @golam-m-hossain:
So you agree that setting this level to 2 gives wrong results. In comment:10, I tried to say that this option that gives wrong results should be removed. We can then merge this inert derivative with the global
|
comment:15
Replying to @burcin:
Yes, but ONLY for mathematically dubious inputs. Please don't generalize, it doesn't help anyone. I have been working on a patch that will check the arguments of the function while applying chain
Sure. However, it would be premature to merge this with global "diff" now, given its a |
comment:16
Replying to @golam-m-hossain:
What do you think is "mathematically dubious"? Using the notation and definitions for derivatives and partial derivatives from here respectively: http://books.google.at/books?id=e54cqeAmf4QC&pg=PA267#v=onepage http://books.google.at/books?id=e54cqeAmf4QC&pg=PA495#v=onepage Let U be an open subset of the complex numbers, f: UxU -> C, y in U. Then by Proposition 3.5 here http://books.google.at/books?id=LzhkCF9ZsUgC&&pg=PA10#v=onepage we have \frac{df}{dy} (y,y) = D_1 f(y, y) + D_2 f(y, y). Note that on the left hand side there is a total derivative. In the current Sage syntax, "diff" denotes a total derivative, and "D" denotes a partial derivative. The statement above translates to the Sage notation as: diff(f(y,y), y) = D0(y,y) + D1(y,y) Which you can also calculate by:
With your patch we get:
Can you explain what
Are you saying that we should merge this problematic version, and you'll fix things later? This is not how the development process works. We can review and merge the known good parts from your patch, and you can submit the rest in a different ticket later.
Should it really be merged into Sage if it's so premature? The point of the review to make sure that it doesn't have these problems. I don't have any more time to waste on this. I suggest you either
|
comment:17
I tend to agree with the points that Burcin made. It seems a better way to implement this within GiNaC would be to have a class parallel to fderivative which just stores the (evaluated) function as well as the list of symbols. It is trivial to go from this data structure to partial derivative one. |
comment:20
We have this now. Close?
|
comment:21
It's hard to tell exactly the scope of this ticket, but from the thread, it seems like one of the primary things wanted was a way to represent
instead of what's in Sage now:
|
comment:22
I think Mike has it correct. (Though this ticket is so old, and involved an unresolved discussion, so that it could conceivably be helpful to start over.) |
Implement a diff format symbolic derivative in new symbolics as the second form of abstract derivative to be avialable in Sage. See this long thread
http://groups.google.com/group/sage-devel/browse_thread/thread/ff10f99729a74eea/73308bf626ae06b3
for rationale behind it.
Implementation:
Instructions for installing these patches (sage-4.1.1)
(1) Pynac patch
(a) Get the pynac spkg
http://sage.math.washington.edu/home/burcin/pynac/pynac-0.1.8.p2.spkg
(b) Apply the pynac patch implementing diff derivative from
http://www.math.unb.ca/~ghossain/diff-derivative-pynac.patch
(c) install the patched spkg in Sage.
OR if you are feeling lazy, you can directly install my patched copy of pynac from here
http://www.math.unb.ca/~ghossain/pynac-0.1.8.p2-with-diff.spkg
(2) Sage patch:
Apply the attached patch in Sage and build the changes ("sage -b").
If everything goes smoothly then you are ready for testing.
Testing:
Above, patches will provide two new user accessible functions
(a)
set_diff_derivative_level
(b)
symbolic_diff
Please see the docs for usage of these functions:
It would be good to thoroughly test the following features:
(1) Substitution of the function
(2) Derivative with or without chain rule
(3) Explicit evaluation of derivative even for some situation where chain rule has been applied and derivative is specified w.r.t. an expression
(4) Symbolic n-th derivative
(5) Typesetting
Please test diff implementation against some related bugs
#6376, #6523, #6480
as new diff implementation should avoid these.
Speed Test:
***** This patch removes old "dummy_diff" from
"calculus/calculus" as we now have a
_real_
diff.CC: @ncalexan @mwhansen @kcrisman
Component: symbolics
Author: Golam Mortuza Hossain
Reviewer: Nick Alexander
Issue created by migration from https://trac.sagemath.org/ticket/6756
The text was updated successfully, but these errors were encountered: