-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Bug fixes on matrix derivatives #17583
Conversation
✅ Hi, I am the SymPy bot (v148). I'm here to help you write a release notes entry. Please read the guide on how to write release notes. Your release notes are in good order. Here is what the release notes will look like: This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.5. Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it. Click here to see the pull request description that was parsed.
Update The release notes on the wiki have been updated. |
Codecov Report
@@ Coverage Diff @@
## master #17583 +/- ##
============================================
- Coverage 74.802% 70.772% -4.03%
============================================
Files 633 633
Lines 164836 164869 +33
Branches 38750 38766 +16
============================================
- Hits 123301 116682 -6619
- Misses 36127 42691 +6564
- Partials 5408 5496 +88 |
function = self.function | ||
if isinstance(function, Lambda) and function.args[0] == (function.args[1],): | ||
# This is a Lambda containing the identity function. | ||
return expr |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lambda has .is_identity
for this. Returning function(expr)
will check that the signature of the function is actually compatible though e.g.:
In [1]: function = Lambda((x, y), x+y)
In [2]: expr = 2
In [3]: function(2)
---------------------------------------------------------------------------
BadArgumentsError
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess a better example would be:
In [7]: function = Lambda((x, y), (x, y))
In [8]: function.is_identity
Out[8]: True
class ElementwiseApplyFunction2(ElementwiseApplyFunction): | ||
def __new__(obj, expr): | ||
class _(ElementwiseApplyFunction): | ||
def __new__(cls, expr): | ||
return ElementwiseApplyFunction(self.function, expr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this isn't being added in this PR but this construction seems like a bad idea. The function should be part of the args rather than messing with func like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, this way you keep the same parameters as in the function definition. Methods applying to functions should also apply here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you give an example of why this is needed? Why can't the class be (basically):
class ElementWiseApplyFunction(MatrixExpr):
def __new__(cls, function, expr):
# Pass function as part of args
obj = MatrixExpr.__new__(cls, function, expr)
A similar construction using classes created on the fly is used for UndefinedFunction
but I think that should be changed as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A generic Python function may not be sympifiable. That's why the argument is hidden.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A generic Python function is turned into a Lambda:
sympy/sympy/matrices/expressions/applyfunc.py
Lines 46 to 51 in fcf84d9
def __new__(cls, function, expr): | |
obj = MatrixExpr.__new__(cls, expr) | |
if not isinstance(function, FunctionClass): | |
d = Dummy("d") | |
function = Lambda(d, function(d)) | |
obj._function = function |
…into matdiff_more_fixes
References to other Issues or PRs
Brief description of what is fixed or changed
Other comments
Release Notes