-
-
Notifications
You must be signed in to change notification settings - Fork 556
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
real nth root function #12074
Comments
Work Issues: needs tests and documentation |
comment:2
Is there any chance this could be made into a function that we can do calculus with, like computing derivatives, integrals, solving equations... (I'm afraid the answer will be no though because we need maxima). |
comment:3
Attachment: trac_12074-nth_root.patch.gz Replying to @jdemeyer:
This is already a symbolic function, so it plays well with symbolics generally (as opposed to piecewise functions for instance):
I updated the patch to add custom exponentiation and derivative methods as well:
This all needs a lot of work of course. For integration and solving equations we call out to maxima. One way to get sensible results from these calls would be to convert this function to a regular |
comment:10
See also sympy's |
comment:11
There is some interest in this again, due to an upcoming book's 2nd edition. Sympy's solution may be more elegant in some ways, because it uses Here's the full Sympy code. They have better handling of complicated piecewise things, I guess.
|
Changed author from Burcin Erocal to Burcin Erocal; Kwankyu Lee |
Branch: public/12074 |
Branch pushed to git repo; I updated commit sha1. New commits:
|
Commit: |
comment:14
Revived Burcin's implementation of It is in public. Feel free to improve. |
Changed work issues from needs tests and documentation to none |
Changed keywords from nth_root to none |
Changed author from Burcin Erocal; Kwankyu Lee to Burcin Erocal, Kwankyu Lee |
comment:36
Replying to @kcrisman:
Injecting the code incur many doctest failures in other places. So I give up this idea. |
comment:37
Replying to @kcrisman:
I see
Done.
I don't get your concern. I think there is nothing wrong with
No. |
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:39
Replying to @kwankyu:
I was thinking of something simpler, like
|
comment:40
Thanks for all these fixes - we are converging :-) Couple more questions/comments:
- sage: plot(real_nth_root(x, 3), (x, -1, 1))
- Graphics object consisting of 1 graphics primitive
+ sage: plot(real_nth_root(x, 3), (x, -1, 1)) # not tested
It's a little tricky, because the humans it is intended for might not realize it's not quite the same. mjo or Nils, any thoughts? I can go with it for now.
Thanks. However, that brings up the related issue that the online documentation is only going to show the If you are up for it, I'd recommend making a more "narrative" structure which includes examples of calculus, plotting, etc., more like real_part does. If you'd prefer me to do that, I can - might just take a little longer as I recall how to add to a public branch :-)
If
Wait, so are you saying that we should disallow that function with |
comment:41
I feel like this would be another ticket, and need sage-devel discussion. This ticket is about adding something to aid in plotting, and then making sure it has enough bells and whistles that it doesn't end up causing confusion. I'm a little uncomfortable with this since
and the point of the function is strictly for pedagogical purposes (which should also be made clear in the documentation). |
comment:42
Replying to @kcrisman:
We run into that same problem whenever we have a subclass method that does something different from its superclass method. I was only trying to clear up my previous suggestion. If the authors/reviewers don't think this is a good idea, feel free to ignore it. |
comment:43
Fair enough.
If this gets merged, why not open a new ticket for an |
comment:44
Replying to @kcrisman:
I'm not 100% convinced it's the right thing to do either. Especially if we already have |
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:46
Replying to @kcrisman:
Done.
Yes. What should
Why is it not correct? It is correct but you should not evaluate 1/x at x=0. |
comment:47
Thanks, I'll check that out momentarily to confirm it builds properly. Looks good so far.
I meant
Depends on whether such expressions are meant to include domains - but anyway that is not directly relevant. |
Reviewer: Karl-Dieter Crisman, Nils Bruin, Kwanyku Lee |
comment:48
Thank you for all the good work - this is really long since overdue.
Again, see comment:21. It is good that
I think if in
that might solve it (suitable tests included). What do you think? Unfortunately the most naive things I tried for the Probably we don't need to |
comment:49
Replying to @kcrisman:
Allowed. This is exactly the same problem with
which we don't bother to fix in any way. If you argue the cases are different because the difference of the domains of definition is of measure zero, then how about this:
Perhaps it is important that these things do not cause real problems because the domain of definition never gets smaller from the initial domain of definition that one starts with. |
comment:50
Okay, I'll roll with that. Haven't heard from any of the other people regarding that here, I don't have a big commitment here since this isn't really switchable to "regular" root functions. Thank you! |
comment:51
Thank you! |
comment:52
Thank you very much for making this happen! |
Changed branch from public/12074 to |
Changed reviewer from Karl-Dieter Crisman, Nils Bruin, Kwanyku Lee to Karl-Dieter Crisman, Nils Bruin, Kwankyu Lee |
Changed commit from |
See sage-devel threads at:
http://groups.google.com/group/sage-devel/t/cea9b562ea49c9c1
and
https://groups.google.com/forum/#!topic/sage-devel/Q8VLKBypcJk
CC: @orlitzky @kcrisman [email protected] @eviatarbach @slel
Component: symbolics
Author: Burcin Erocal, Kwankyu Lee
Branch:
f8cb7a0
Reviewer: Karl-Dieter Crisman, Nils Bruin, Kwankyu Lee
Issue created by migration from https://trac.sagemath.org/ticket/12074
The text was updated successfully, but these errors were encountered: