Skip to content
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

Sympy standard function with our types #984

Closed
mloubout opened this issue Oct 25, 2019 · 8 comments · Fixed by #1549
Closed

Sympy standard function with our types #984

mloubout opened this issue Oct 25, 2019 · 8 comments · Fixed by #1549
Assignees
Labels

Comments

@mloubout
Copy link
Contributor

So basically the issue is that all of sympy standard functions ignore our type or transform into their own

For example sqrt(u) is a sympy.Pow while we would want a Differentiable.Pow. I am unsure how to proceed about this, most of the ideas I would require to import everything in Devito and add our own version and would break the moment someone uses a function we didn;t think of (there is like 100 in sympy so not gonna redo everything)

Open to suggestions

@FabioLuporini
Copy link
Contributor

I think the only possibility is to import them all and subclass them such that they get a different new just like Add, Mul, ...

I don't think it's bad, it's just a little tedious but it shouldn't take an insane amount of time...

I'd add all nonlinear functions here: https://fenicsproject.org/docs/ufl/1.4.0/ufl.html#id2

it's roughly more than 20, so it's doable

@mloubout
Copy link
Contributor Author

Yes it is just tidious I don't mind that way just takes time

@CavalcanteLucas
Copy link
Contributor

CavalcanteLucas commented Oct 28, 2019

sympy/sympy/functions/elementary/miscellaneous.py

  • Max
    class: L#642
  • Min
    class: L#760
  • sqrt
    function: L#60

sympy/sympy/functions/elementary/complexes.py

  • abs
    class: L#400
  • sign
    class: L#246

sympy/functions/elementary/exponential.py

  • exp
    class: L#190
  • ln

sympy/functions/special/error_functions.py

  • erf
    class: L#44

sympy/functions/elementary/trigonometric.py

  • cos
    class: L#495
  • sin
    class: L#203
  • tan
    class: L#931
  • acos
    class: L#2206
  • asin
    class: L#2039
  • atan
    class: L#2379
  • atan_2
    class: L#2976

sympy/functions/elementary/hyperbolic.py

  • cosh
    class: L#264
  • sinh
    class: L#69
  • tanh
    class: L#432

@FabioLuporini
Copy link
Contributor

@btrb would you like to give this a go? as a simple self-contained exercise?

@CavalcanteLucas
Copy link
Contributor

oh, yes. sure! although i don't quite have a clear picture of a solution
one thing that crossed my mind, let's say for Max, was to:
declare a new class in differentiable.py:

class Max(sympy.Max, DifferentiableOp):
    __new__ = DifferentiableOp.__new__

add it to class diffiffy, like:

    @_cls.register(sympy.Max)
    def _(obj):
        return Max

add the corresponding decorator to diffify._():

@_cls.register(Max)

then include

evalf_table[Max] = evalf_table[sympy.Max]

and finally override the sympy operator in Differentiable with

    def __max__(self, other):
        return Max(self, other)

i'm aware that it should not be that trivial though

@mloubout
Copy link
Contributor Author

I would avoid to go through diffify too much and clutter it with too much stuff. Most of these are sympy.Function so there should be a way to go through our own basic to make it nicely and avoid too much code.

@mloubout
Copy link
Contributor Author

mloubout commented Nov 6, 2019

This "update" would also be a good time to enforce types on these functions in the generated code (cos/cosf, sin/sinf,....) as we currently have the double precisions ones (cos, sin,...) always (unless I missed something)

mloubout added a commit that referenced this issue Apr 14, 2021
@mloubout
Copy link
Contributor Author

Started in #1549 , not all of it though, will need to add the other ones the same way.

mloubout added a commit that referenced this issue Apr 14, 2021
mloubout added a commit that referenced this issue Apr 15, 2021
mloubout added a commit that referenced this issue Apr 16, 2021
mloubout added a commit that referenced this issue Apr 22, 2021
mloubout added a commit that referenced this issue Apr 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants