Opened 11 years ago

Closed 8 years ago

#12922 closed enhancement (fixed)

Implicit derivative

Reported by: Miguel Marco Owned by: Burcin Erocal
Priority: major Milestone: sage-6.5
Component: calculus Keywords: implicit derivative
Cc: Karl-Dieter Crisman, Samuel Gonshaw, Burcin Erocal Merged in:
Authors: Miguel Marco Reviewers: Kannappan Sampath, Ralf Stephan
Report Upstream: N/A Work issues:
Branch: 0a76da7 (Commits, GitHub, GitLab) Commit: 0a76da78806d27c25e8a5864c26b3106611ab940
Dependencies: Stopgaps:

Status badges

Description

Added implicit_derivative function. It allows to compute the n'th derivative of the implicit function defined by a symbolic expression.

Attachments (3)

16191.patch (1.9 KB) - added by Miguel Marco 11 years ago.
16191_review.patch (2.3 KB) - added by Samuel Gonshaw 10 years ago.
adds error handler
12922.patch (14.6 KB) - added by Miguel Marco 9 years ago.
Newer version, solves the pynac registry pollution without manipulating it

Download all attachments as: .zip

Change History (39)

Changed 11 years ago by Miguel Marco

Attachment: 16191.patch added

comment:1 Changed 11 years ago by Miguel Marco

Status: newneeds_review

comment:2 Changed 11 years ago by Karl-Dieter Crisman

Cc: Karl-Dieter Crisman added

comment:3 Changed 10 years ago by Samuel Gonshaw

Reviewed, seems good. Added a check for having no y term in the expression. Was previously an uncaught DivideByZero, while code now gives a more informative error. Was my first review, would appreciate to know whether I'm actually being any help!

comment:4 Changed 10 years ago by Karl-Dieter Crisman

Yes, both helpful! A few comments.

  • The reviewer patch should be a separate patch on top of the original patch, so that the original author is kept.
  • In any case, the patch should have a meaningful commit message.
  • I'm a little apprehensive about the many specific variables and functions defined here. Won't that cause some kind of conflict with (say) an already-defined yy? We shouldn't depend on people not needing those variables. Or is everything only in local scope because of the use of SR.symbol?

I'm also wondering - is it possible to have an implicit derivative method for symbolic expressions? Just curious.

Changed 10 years ago by Samuel Gonshaw

Attachment: 16191_review.patch added

adds error handler

comment:5 Changed 10 years ago by Miguel Marco

All the variables defined in the function are local. They do not get mixed with the global variables with the same name. You can even call the function with xx and yy as parameters, and they will not be mixed with the local xx and yy.

sage: var('x,y')
(x, y)
sage: implicit_derivative(y,x,y*x/cos(x+y))
-(x*y*sin(x + y)/cos(x + y)^2 + y/cos(x + y))/(x*y*sin(x + y)/cos(x + y)^2 + x/cos(x + y))
sage: var('xx,yy')
(xx, yy)
sage: implicit_derivative(yy,xx,yy*xx/cos(xx+yy))
-(xx*yy*sin(xx + yy)/cos(xx + yy)^2 + yy/cos(xx + yy))/(xx*yy*sin(xx + yy)/cos(xx + yy)^2 + xx/cos(xx + yy))

About writing this as a method for symbolic expressions... that was my first idea; but i prefeared this aproach, due to the fact that you are actually not diferentiating the symbolic expression, but a function defined implicitely by the symbolic expression. So it seemed more natural to me this kind of syntax.

comment:6 Changed 10 years ago by Samuel Gonshaw

Calling var or SR.symbol('somename') even locally does add to the pynac_symbol_registry, yet calling SR.symbol() with no argument will create a temporary variable that will not be added to this registry. Possibly adding to the registry may cause problems if it is being referenced directly for some reason. Maybe worth just using x = SR.symbol() etc?

sage: from sage.symbolic.ring import pynac_symbol_registry
sage: pynac_symbol_registry                               
{'some_variable': some_variable, 'A': A, 'C': C, 'B': B, 'E': E, 'D': D, 'G': G, 'F': F, 'H': H, 'K': K, 'J': J, 'M': M, 'L': L, 'N': N, 'Q': Q, 'P': P, 'S': S, 'R': R, 'U': U, 'T': T, 'W': W, 'V': V, 'Y': Y, 'X': X, 'Z': Z, 'a': a, 'c': c, 'b': b, 'd': d, 'g': g, 'f': f, 'h': h, 'k': k, 'j': j, 'm': m, 'l': l, 'o': o, 'n': n, 'q': q, 'p': p, 's': s, 'r': r, 'u': u, 't': t, 'w': w, 'v': v, 'y': y, 'x': x, 'z': z}
sage: def f(): temp = SR.symbol()                         
....: 
sage: f()
sage: pynac_symbol_registry
{'some_variable': some_variable, 'A': A, 'C': C, 'B': B, 'E': E, 'D': D, 'G': G, 'F': F, 'H': H, 'K': K, 'J': J, 'M': M, 'L': L, 'N': N, 'Q': Q, 'P': P, 'S': S, 'R': R, 'U': U, 'T': T, 'W': W, 'V': V, 'Y': Y, 'X': X, 'Z': Z, 'a': a, 'c': c, 'b': b, 'd': d, 'g': g, 'f': f, 'h': h, 'k': k, 'j': j, 'm': m, 'l': l, 'o': o, 'n': n, 'q': q, 'p': p, 's': s, 'r': r, 'u': u, 't': t, 'w': w, 'v': v, 'y': y, 'x': x, 'z': z}
sage: def f(): temp = SR.symbol('temp')
....: 
sage: f()
sage: pynac_symbol_registry
{'temp': temp, 'some_variable': some_variable, 'A': A, 'C': C, 'B': B, 'E': E, 'D': D, 'G': G, 'F': F, 'H': H, 'K': K, 'J': J, 'M': M, 'L': L, 'N': N, 'Q': Q, 'P': P, 'S': S, 'R': R, 'U': U, 'T': T, 'W': W, 'V': V, 'Y': Y, 'X': X, 'Z': Z, 'a': a, 'c': c, 'b': b, 'd': d, 'g': g, 'f': f, 'h': h, 'k': k, 'j': j, 'm': m, 'l': l, 'o': o, 'n': n, 'q': q, 'p': p, 's': s, 'r': r, 'u': u, 't': t, 'w': w, 'v': v, 'y': y, 'x': x, 'z': z}
Last edited 10 years ago by Samuel Gonshaw (previous) (diff)

comment:7 Changed 10 years ago by Miguel Marco

I have tried to use SR.symbol() instead of SR.symbol('x'), but then the result is a mess.

I can't think of a way to make this work without the said side effect.

comment:8 Changed 9 years ago by Miguel Marco

Cc: Samuel Gonshaw added

I solved the pynac symbol registry pollution by saving a copy of it at the beginning of the function, and restoring it at the end. I am not sure if this is the best choice, but it is the only solution i found, and seems to work fine.

comment:9 Changed 9 years ago by Burcin Erocal

Status: needs_reviewneeds_work

Messing with the symbol registry this way is really not a good idea. That is just supposed to be internal data.

What if there is an exception in that code and the registry is not restored?

What exactly is the problem with using temporary variables? Looking briefly at the patch, it looks like we also need temporary functions.

comment:10 Changed 9 years ago by Miguel Marco

The problem with using temporary variables without giving them a name string is this:

sage: from sage.symbolic.function_factory import SymbolicFunction
sage: var('x,y')                                                                                                                                    
(x, y)
sage: def implicit_derivative(Y,X,F,n=1):                                                                                                           
....:         x=SR.symbol()                                                                                                                         
....:         yy=SR.symbol()                                                                                                                        
....:         y=SymbolicFunction('y',1)(x)                                                                                                          
....:         f=SymbolicFunction('f',2)(x,yy)                                                                                                       
....:         Fx=f.diff(x)                                                                                                                          
....:         Fy=f.diff(yy)                                                                                                                         
....:         G=-(Fx/Fy)                                                                                                                            
....:         G=G.subs_expr({yy:y})                                                                                                                 
....:         di={y.diff(x):-F.diff(X)/F.diff(Y)}                                                                                                   
....:         R=G                                                                                                                                   
....:         S=G.diff(x,n-1)                                                                                                                       
....:         for i in range(n+1):                                                                                                                  
....:                 di[y.diff(x,i+1)(x=x)]=R                                                                                                      
....:                 S=S.subs_expr(di)                                                                                                             
....:                 R=G.diff(x,i)                                                                                                                 
....:                 for j in range(n+1-i):                                                                                                        
....:                         di[f.diff(x,i,yy,j)(x=x,yy=y)]=F.diff(X,i,Y,j)                                                                        
....:                         S=S.subs_expr(di)                                                                                                     
....:                 return S                                                                                                                      
....:                                                                                                                                               
sage: implicit_derivative(y,x,x^2+y^2-1)                                                                                                            
-D[0](f)(symbol160, y(symbol160))/D[1](f)(symbol160, y(symbol160))

I don't know why, this problem is not present if we put the strings in the definitions of the variables.

comment:11 in reply to:  10 Changed 9 years ago by Burcin Erocal

Replying to mmarco:

sage: implicit_derivative(y,x,x^2+y^2-1)                                                                                  
-D[0](f)(symbol160, y(symbol160))/D[1](f)(symbol160, y(symbol160))

It looks like the substitution doesn't work at some point.

Looking at the code...

The calls y.diff(x,i+1)(x=x) and f.diff(x,i,yy,j)(x=x,yy=y) rely on the name of the variable. You should use a dictionary argument to .subs().

BTW, the subs_expr() method will go away at some point. Why not just use .subs() everywhere?

comment:12 Changed 9 years ago by Burcin Erocal

Before this gets switched to positive review, we should check how it behaves if there already is a symbolic function defined with a custom method for evaluation named y or f.

Why is this not a method of symbolic expressions? Isn't it inconsistent to have the functional notation supported but not a class method?

comment:13 Changed 9 years ago by Miguel Marco

Thanks for the comments. I will try to work on it.

As i said, the reason why it is not a method is that it is not clear to me what class should it be in.

Is this a method of the expression F? Well, not exactly, since F is not the function that we are trying to differentiate, but the symbollic expression that deffines it implicitly.

Of X? This is just the variable with which we differentiate on.

Of Y? Riguorilously that should be the function that we are differentiating. But doesn't sound natural at all to add a method like this for symbollic variables.

Or maybe we could define a new class ImplicitelyDefinedSymbolicFunction? for this case, bat that sounds way overkill.

Changed 9 years ago by Miguel Marco

Attachment: 12922.patch added

Newer version, solves the pynac registry pollution without manipulating it

comment:14 Changed 9 years ago by Miguel Marco

Ok, your suggestions seemed to work fine. I have uploaded a newer version. Please check it.

comment:15 Changed 9 years ago by Miguel Marco

Status: needs_workneeds_review

comment:16 Changed 9 years ago by Jeroen Demeyer

Milestone: sage-5.11sage-5.12

comment:17 Changed 9 years ago by For batch modifications

Milestone: sage-6.1sage-6.2

comment:18 Changed 9 years ago by For batch modifications

Milestone: sage-6.2sage-6.3

comment:19 Changed 8 years ago by For batch modifications

Milestone: sage-6.3sage-6.4

comment:20 Changed 8 years ago by Miguel Marco

Cc: Burcin Erocal added

bump

comment:21 Changed 8 years ago by Kannappan Sampath

Hi mmarco:

Thanks for writing up a patch regarding something very fundamental to calculus. Sage has since v6.0 moved to Git for its revision control. As a consequence, we work with "branches" instead of "patches" and so on. Could you kindly make the patch here into a branch?

I think this might be a bit painful depending on several factors. So, please feel free to ask for any help!

comment:22 Changed 8 years ago by Kannappan Sampath

Status: needs_reviewneeds_work

comment:23 Changed 8 years ago by Miguel Marco

Branch: u/mmarco/ticket/12922
Created: May 8, 2012, 11:35:00 AMMay 8, 2012, 11:35:00 AM
Modified: Aug 23, 2014, 12:10:47 PMAug 23, 2014, 12:10:47 PM

comment:24 Changed 8 years ago by Kannappan Sampath

Commit: 39aaf0c4b33a7a5ec0066e64e25cf5e54d197247
Reviewers: Kannappan Sampath
Status: needs_workneeds_review

New commits:

39aaf0cAdded implicit_derivative

comment:25 Changed 8 years ago by Kannappan Sampath

For starters, I have the following comments:

  1. The one-line docstring for the function is "incorrect": it computes something, yes, but it also returns it. So, please change that to "Return <whatever>".
  2. The value error could return the expression F explicitly instead of saying the generic F. While you are it, please use the .format to format the string for the output instead.

Honestly, I have not looked at the math. I will get back about the math and the programming part in a bit.

comment:26 Changed 8 years ago by git

Commit: 39aaf0c4b33a7a5ec0066e64e25cf5e54d1972473f3f949cff46cc4e2f5b93bcf41de4c3f81f13cd

Branch pushed to git repo; I updated commit sha1. New commits:

3f3f949Reviewer's suggestions changed, plus some changes for PEP8 compliance.

comment:27 in reply to:  13 Changed 8 years ago by Ralf Stephan

Replying to mmarco:

As i said, the reason why it is not a method is that it is not clear to me what class should it be in.

Is this a method of the expression F? Well, not exactly, since F is not the function that we are trying to differentiate, but the symbollic expression that deffines it implicitly.

I think Burcin wants this method part of the symbolic expression or function class. If it's not tied to a specific expression then make it a @classmethod (equivalent to a static method in C), and then create the globally accessible name for it.

comment:28 Changed 8 years ago by git

Commit: 3f3f949cff46cc4e2f5b93bcf41de4c3f81f13cd5fe0ffeca2be3ec698b7eccfeb51ce80fa94a62c

Branch pushed to git repo; I updated commit sha1. New commits:

5fe0ffeMoved to a method in the expression class

comment:29 Changed 8 years ago by Miguel Marco

Moved it to a method in the expression class.

comment:30 Changed 8 years ago by Ralf Stephan

Branch: u/mmarco/ticket/12922u/rws/ticket/12922

comment:31 Changed 8 years ago by Ralf Stephan

Commit: 5fe0ffeca2be3ec698b7eccfeb51ce80fa94a62c954997662dc64a089c3ce6c90d423a78f3117834
Milestone: sage-6.4sage-6.5
Reviewers: Kannappan SampathKannappan Sampath, Ralf Stephan
Status: needs_reviewpositive_review

Just some minor fixes. It looks good. I did no make ptestlong as it's just a new method.


New commits:

7e00610Merge branch 'develop' into t/12922/ticket/12922
954997612922 reviewer's patch: optimize imports and doc cosmetics

comment:32 Changed 8 years ago by Miguel Marco

Thanks for the review.

comment:33 Changed 8 years ago by Marc Mezzarobba

Status: positive_reviewneeds_work
**********************************************************************
File "src/sage/misc/sageinspect.py", line 1815, in sage.misc.sageinspect.sage_getsourcelines
Failed example:
    sage_getsourcelines(x)[0][-1]    # last line
Expected:
    '        return self / x\n'
Got:
    '        return S\n'
**********************************************************************

comment:34 Changed 8 years ago by git

Commit: 954997662dc64a089c3ce6c90d423a78f31178340a76da78806d27c25e8a5864c26b3106611ab940

Branch pushed to git repo; I updated commit sha1. New commits:

0a76da712922: fix doctest

comment:35 Changed 8 years ago by Ralf Stephan

Status: needs_workpositive_review

Interesting. That doctest depends on the last line in class Expression to remain unchanged.

comment:36 Changed 8 years ago by Volker Braun

Branch: u/rws/ticket/129220a76da78806d27c25e8a5864c26b3106611ab940
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.