Addition of Directional derivatives #12417

mohit3011 · 2017-03-25T07:42:04Z

…atives

mohit3011 · 2017-03-25T17:22:30Z

@jksuom , @asmeurer please review this PR . Thanks

szymag · 2017-03-27T19:14:35Z

sympy/vector/functions.py

@@ -214,6 +214,38 @@ def gradient(scalar, coord_sys):

    return coord_sys.delop(scalar).doit()

+def directional_derivative(scalar, vect, coord_sys):
+    """
+    Returns the directional derivative of a scalar field computed along a  given vector


Here you have double white space, after "along a".
If you want to correct this, you shouldn't push new commit. You can use git commit --amend. It overrides before one.

@szymag , thanks for reviewing it. I will correct the mistake.

…atives

…1/sympy into Directional_derivatives

szymag · 2017-03-27T21:35:09Z

I have some doubts about your solution.

I think that vector, which you take as parameter, should be normalized. Here, you can find some information about it (here or in the next film):
https://www.khanacademy.org/math/multivariable-calculus/multivariable-derivatives/gradient-and-directional-derivatives/v/directional-derivative

Directional derivative is a function, but you return vector. More explicitly <class 'sympy.core.add.Add'>. It may be complicated for user, who also want to calculate value of functional derivative in some point.

What do you think?

mohit3011 · 2017-03-27T21:43:11Z

@szymag , I don't think that normalization is necessary. The definition of Directional Derivatives as given here Wikipedia considers Directional Derivative as gradient(f).v.

Also , the function here doesn't return a vector , rather it returns a dot product which is always scalar.

mohit3011 · 2017-03-27T21:46:54Z

Also quoting from Wikipedia's Definition

In the context of a function on a Euclidean space, some texts restrict the vector v to being a unit vector. Without the restriction, this definition is valid in a broad range of contexts.

Therefore I have not kept the condition of V being a unit vector so that it could be used in broader sense.

szymag · 2017-03-27T22:01:11Z

You always should allowed user to put any vector, but then normalized them.
My argumentation here is that, when you put two different vector with the same direction but different length, you get different result.

I'm not going to persist on my idea, it is just convention. It will be better, if someone from maintainers takes a look.

mohit3011 · 2017-03-28T19:02:10Z

ping @jksuom @asmeurer @Upabjojr , please review this PR

siefkenj · 2017-03-28T22:14:36Z

The directional derivative is unfortunately named, but you should allow any vector (including the zero vector) as input. That way, the directional derivative is a linear function in its direction for differentiable functions. Some textbooks work around the issue by defining the directional derivative only for unit vectors and leaving it the term "directional derivative" unspecified for non-unit vectors.

However, your code will only produce the directional derivative for a differentiable function. The true definition of the directional derivative of f at the point x in the direction v is lim_{h -> 0}( f(x+hv)-f(x))/h which is only equal to grad f(x) . v if f is differentiable. Does sympy have a differentiability test?

mohit3011 · 2017-03-29T07:12:44Z

@siefkenj

The directional derivative is unfortunately named, but you should allow any vector (including the zero vector) as input.

As you said this functions allows any vector as input.

directional derivative of f at the point x in the direction v is lim_{h -> 0}( f(x+hv)-f(x))/h which is only equal to grad f(x) . v if f is differentiable.

I have used the existing Gradient function in Sympy for DIrectional Derivative so it must be checking the conditions of differentiability as Gradient function further calls the existing Derivative function of Sympy.

Upabjojr · 2017-03-29T12:30:26Z

sympy/vector/functions.py

+    5*R.x**2 + 30*R.x*R.z
+
+    """
+    return gradient(scalar,coord_sys).dot(vect).doit()


This is unrelated to this PR, but I believe gradient should be called as gradient(scala) without the coord_sys variable.

Not really, look at documentation:
http://docs.sympy.org/dev/modules/vector/fields.html
We need manually put information about coordinate system. For me it is also strange

I know that, that's why we need to modify the gradient function.

Sorry, I misunderstand your comment a bit.

I know that, that's why we need to modify the gradient function.

I agree with you, but now it isn't so easy. If I correctly understand code, vector has no information about his coordinate system, we need forward this information explicitly to function (like gradient).
I am writing proposal, where I also include this information, and I would like to fix this issue.

@Upabjojr , It won't be that easy now as this would mean changing the whole sytem of deloperator.py and functions.py. So it's better to let it be in this way.

Yes, I wrote that this issue is not related to this PR.

If I correctly understand code, vector has no information about his coordinate system,

It has:

In [6]: c = CoordSysCartesian('rect') In [7]: c Out[7]: rect In [8]: c.x Out[8]: rect_x In [9]: c.x.args Out[9]: (rect.x, 0, rect, rectₓ, \mathbf{{x}_{rect}}) In [10]: c.x.args[2] Out[10]: rect In [11]: c.x.args[2] == c Out[11]: True

@Upabjojr , in reply to your last comment. I would like to point out that in whenever we call the gradient function from vector/functions.py then it calls the class CoordSysCartesian in vector/coordsysrect.py int his class delop function is called. which further calls Del function in deloperator.py . Now, the class CoordSysCartesian requires the name of Coordinate system for assigning purposes so that's we are passing coordinate system in gradient function.

Yeah, it needs some cleanup.

@Upabjojr , please merge this PR. Thanks

Upabjojr · 2017-03-29T19:39:10Z

sympy/vector/functions.py

+    >>> R = CoordSysCartesian('R')
+    >>> f1 = R.x*R.y*R.z
+    >>> v1 = 3*R.i + 4*R.j + R.k
+    >>> directional_derivative(f1, v1, R)


It's the same problem as in the gradient function. I don't like the R parameter in the API.

@Upabjojr , I understand what you mean, but as I told you earlier it would be a very large amount of work to clean up the whole coordinate system with each function. So for the existing system this is the best possible way

Maybe the work for redefining the whole system can be done in other PRs

No, it's simple. Use .atoms( ).

What does .atoms() does ? and how it will help here ?

Maybe you can also extract the coordinate system object from vector.

@Upabjojr , maybe we can extract that information but then it would create a problem that it would be out of convention. What I mean is that in the existing system the user has to to give cordinate system as input in every function , now if I create a function where user doesn't have to give the coordinate system as input then it would create confusion and disturb the conventions.

Every time user wishes to use this function then he/she has to use a syntax which would be different from other functions.

Let's start writing the new functions with an intuitive syntax. This can be fixed by adding one line to extract the coordinate system and then calling the gradient.

Okay , let's do it

…atives

Upabjojr · 2017-03-31T10:28:32Z

vector.atoms(CoordSysCartesian)

…atives

mohit3011 · 2017-03-31T12:02:38Z

@Upabjojr , it has passed all Tests except one which has error due to exceeded time limit.

mohit3011 · 2017-03-31T14:41:55Z

@Upabjojr , please review this PR thanks

mohit3011 · 2017-03-31T17:27:09Z

@Upabjojr , Thanks

mohit3011 added 5 commits March 25, 2017 13:03

Add Directional Derivative function

ce248ed

Add the tests

302390b

Remove the white spaces and correct one example

bfef498

Merge remote-tracking branch 'upstream/master' into Directional_deriv…

b3c72f0

…atives

Correct examples

c155824

szymag reviewed Mar 27, 2017

View reviewed changes

mohit3011 added 3 commits March 28, 2017 03:02

Correct Examples

b59510c

Merge remote-tracking branch 'upstream/master' into Directional_deriv…

8a0e986

…atives

Merge branch 'Directional_derivatives' of https://github.com/mohit301…

e08d0f1

…1/sympy into Directional_derivatives

Upabjojr reviewed Mar 29, 2017

View reviewed changes

mohit3011 added 2 commits March 30, 2017 01:50

Merge remote-tracking branch 'upstream/master' into Directional_deriv…

99734b1

…atives

Merge remote-tracking branch 'upstream/master' into Directional_deriv…

fdd99e9

…atives

mohit3011 added 2 commits March 31, 2017 16:18

Merge remote-tracking branch 'upstream/master' into Directional_deriv…

fe1475a

…atives

Remove dependency of Cartesian System

455306b

Upabjojr merged commit b921ef3 into sympy:master Mar 31, 2017

mohit3011 deleted the Directional_derivatives branch March 31, 2017 17:26

szymag mentioned this pull request Jul 2, 2017

Update documentation. #12869

Closed

szymag mentioned this pull request Jul 2, 2017

Update documentation #12871

Merged

szymag mentioned this pull request Jul 18, 2017

Reconstruction of directional derivative #12991

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Addition of Directional derivatives #12417

Addition of Directional derivatives #12417

mohit3011 commented Mar 25, 2017

mohit3011 commented Mar 25, 2017

szymag Mar 27, 2017

mohit3011 Mar 27, 2017

szymag commented Mar 27, 2017

mohit3011 commented Mar 27, 2017 •

edited

Loading

mohit3011 commented Mar 27, 2017

szymag commented Mar 27, 2017

mohit3011 commented Mar 28, 2017 •

edited

Loading

siefkenj commented Mar 28, 2017

mohit3011 commented Mar 29, 2017

Upabjojr Mar 29, 2017

szymag Mar 29, 2017

Upabjojr Mar 29, 2017

szymag Mar 29, 2017

mohit3011 Mar 29, 2017 •

edited

Loading

Upabjojr Mar 29, 2017

Upabjojr Mar 29, 2017

mohit3011 Mar 29, 2017

Upabjojr Mar 29, 2017

mohit3011 Mar 29, 2017

Upabjojr Mar 29, 2017

mohit3011 Mar 29, 2017

mohit3011 Mar 29, 2017 •

edited

Loading

Upabjojr Mar 29, 2017

mohit3011 Mar 29, 2017 •

edited

Loading

Upabjojr Mar 29, 2017

mohit3011 Mar 29, 2017

mohit3011 Mar 29, 2017

Upabjojr Mar 29, 2017

mohit3011 Mar 29, 2017

Upabjojr commented Mar 31, 2017

mohit3011 commented Mar 31, 2017

mohit3011 commented Mar 31, 2017

mohit3011 commented Mar 31, 2017

Addition of Directional derivatives #12417

Addition of Directional derivatives #12417

Conversation

mohit3011 commented Mar 25, 2017

mohit3011 commented Mar 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

szymag commented Mar 27, 2017

mohit3011 commented Mar 27, 2017 • edited Loading

mohit3011 commented Mar 27, 2017

szymag commented Mar 27, 2017

mohit3011 commented Mar 28, 2017 • edited Loading

siefkenj commented Mar 28, 2017

mohit3011 commented Mar 29, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mohit3011 Mar 29, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mohit3011 Mar 29, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mohit3011 Mar 29, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Upabjojr commented Mar 31, 2017

mohit3011 commented Mar 31, 2017

mohit3011 commented Mar 31, 2017

mohit3011 commented Mar 31, 2017

mohit3011 commented Mar 27, 2017 •

edited

Loading

mohit3011 commented Mar 28, 2017 •

edited

Loading

mohit3011 Mar 29, 2017 •

edited

Loading

mohit3011 Mar 29, 2017 •

edited

Loading

mohit3011 Mar 29, 2017 •

edited

Loading