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

Mixed coordinates for gradient #13118

Merged
merged 5 commits into from Aug 14, 2017

Conversation

Projects
None yet
2 participants
@szymag
Contributor

szymag commented Aug 12, 2017

This PR allows gradient to handle mixed coordinate system.
Not every new tests could be unlocked, because at that moment Gradient, unevaluated expression, isn't interpret as vector so we cannot add such expression to another vector.

TODO:

  • Gradient need to be interpreted as vector (#13097).

szymag added some commits Aug 12, 2017

Create split_coordinate function.
This function is resposible for spliting mul expression
into element by their coordinate system.
@szymag

This comment has been minimized.

Show comment
Hide comment
@szymag

szymag Aug 12, 2017

Contributor

ping @Upabjojr

Contributor

szymag commented Aug 12, 2017

ping @Upabjojr

Show outdated Hide outdated sympy/vector/functions.py Outdated
Show outdated Hide outdated sympy/vector/operators.py Outdated
Show outdated Hide outdated sympy/vector/operators.py Outdated
Show outdated Hide outdated sympy/vector/operators.py Outdated
Show outdated Hide outdated sympy/vector/operators.py Outdated
Show outdated Hide outdated sympy/vector/operators.py Outdated
@Upabjojr

This comment has been minimized.

Show comment
Hide comment
@Upabjojr

Upabjojr Aug 13, 2017

Contributor

I was expecting to have an addition of 2-3 lines to the gradient( ) code. Could you please write a recursive function?

Contributor

Upabjojr commented Aug 13, 2017

I was expecting to have an addition of 2-3 lines to the gradient( ) code. Could you please write a recursive function?

@Upabjojr

This comment has been minimized.

Show comment
Hide comment
@Upabjojr

Upabjojr Aug 13, 2017

Contributor

I would just put an if at the beginning of the function, for the case where there are many coordinate systems. Just this addition.

Contributor

Upabjojr commented Aug 13, 2017

I would just put an if at the beginning of the function, for the case where there are many coordinate systems. Just this addition.

Show outdated Hide outdated sympy/vector/operators.py Outdated
if isinstance(scalar_field, (Mul, VectorMul)):
s = _split_mul_args_wrt_coordsys(scalar_field)
return VectorAdd.fromiter(scalar_field / i * gradient(i) for i in s)
return Gradient(scalar_field)

This comment has been minimized.

@Upabjojr

Upabjojr Aug 13, 2017

Contributor

@szymag this is what I meant. One case for the addition, and another one for the multiplication. It looks a lot more readable.

@Upabjojr

Upabjojr Aug 13, 2017

Contributor

@szymag this is what I meant. One case for the addition, and another one for the multiplication. It looks a lot more readable.

This comment has been minimized.

@szymag

szymag Aug 13, 2017

Contributor

It's very good solution. I didn't know that I can call method inside method itself.
I used to made recursion only with yield

@szymag

szymag Aug 13, 2017

Contributor

It's very good solution. I didn't know that I can call method inside method itself.
I used to made recursion only with yield

This comment has been minimized.

@Upabjojr

Upabjojr Aug 13, 2017

Contributor

If course you can, it's a valid function call.

@Upabjojr

Upabjojr Aug 13, 2017

Contributor

If course you can, it's a valid function call.

@Upabjojr Upabjojr merged commit e8a3808 into sympy:master Aug 14, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@szymag szymag deleted the szymag:mixed_coordinates branch Aug 24, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment