search

pymc-devs / pytensor

PyTensor allows you to define, optimize, and efficiently evaluate mathematical expressions involving multi-dimensional arrays.
https://pytensor.readthedocs.io
Other
325 stars 97 forks source link

Include Op in message when raising NotImplementedError from grad #711

Open ricardoV94 opened 4 months ago

ricardoV94 commented 4 months ago

Description

When grad fails because a specific Op doesn't support it it's quite difficult to figure out which one it was without jumping on an interactive debugger

https://discourse.pymc.io/t/avoiding-non-nuts-samplers-when-broadcasting-a-multiplication-operation/14245/4?u=ricardov94

We should return something like NotImplementedError(f"grad not implemented for Op {self}") in a base class.

Or use the grad_not_implemented functionality that exists for partial grad implementations which also provides a more readable error IIRC

tanish1729 commented 4 months ago

hello! i'll take up this issue. i understand the issue and also saw the main issue on pymc. how should i start solving this?

ricardoV94 commented 4 months ago

You should tweak this base case NotImplementedError, to include self in the message: https://github.com/pymc-devs/pytensor/blob/090b844ece5412535ab3d030c5bfc2d168aba2ef/pytensor/graph/op.py#L360

tanish1729 commented 4 months ago

Im trying to use the grad_not_implemented function but im slightly confused about the implementation. what is the inputs parameter to the grad function so that i can use it while passing to the grad_not_implemented as used in some other places.

tanish1729 commented 4 months ago

i made some commits to my fork but they arent showing up in this issue for u to review? :confused:

ricardoV94 commented 4 months ago

You need to open a pull request from your fork: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request-from-a-fork