dsl: JDBetteridge/derivative hotfix - #2714
Conversation
Sorry, something went wrong.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #2714 +/- ##
=======================================
Coverage 82.94% 82.95%
=======================================
Files 248 248
Lines 50022 50051 +29
Branches 4402 4406 +4
=======================================
+ Hits 41492 41521 +29
Misses 7776 7776
Partials 754 754
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry.
|
Sorry, something went wrong.
| """ | ||
| if type(expr) is sympy.Derivative: | ||
| raise ValueError("Cannot nest sympy.Derivative with devito.Derivative") | ||
| raise ValueError('Cannot nest sympy.Derivative with devito.Derivative') |
There was a problem hiding this comment.
ultra nitpick: We tend to use double quotes for string messages
Sorry, something went wrong.
There was a problem hiding this comment.
This seems inconsistent with all other strings, is there a good reason?
Sorry, something went wrong.
|
I think this is ready to go, I have addressed the reviewer comments and Mathias' concerns from Slack |
Sorry, something went wrong.
| The following should all be instantiatible without raising an | ||
| exception, but should not simplify. | ||
| """ | ||
| assert Derivative(self.x, self.t) |
There was a problem hiding this comment.
so what happens at .evaluate here since the order is like 99?
Sorry, something went wrong.
There was a problem hiding this comment.
I'm not sure if you worked it out before this was merged, but you will get a value error.
This is because the Dimension is not Differentiable. I'm not sure this was supported before, so I don't think it will cause any issues. We can revisit this if it is something we actually want to support, but I think the most important cases are covered. Specifically simplifying dx/dx to 1 and (d^n/dx^n) x to 0.
Sorry, something went wrong.
I think we should be able to differentiate a dimension wrt the same dimension and get 1, but that's about it.