remove clone in _compute_jacobian_wrt_params_with_sample_wise_trick
#1111
Conversation
Summary: In https://www.internalfb.com/code/fbsource/[76c57d350097]/fbcode/pytorch/captum/captum/_utils/gradient.py?lines=840-841 we are calling `clone` on the elements of `inputs`. However, since `inputs` is of type `Tuple[Any,...]`, those elements may not be Tensors, and thus may not have the clone method. My understanding is that we call `clone` because in `apply_gradient_requirements`, we change `requires_grad=True` for all elements of `inputs`. Even if those elements were Tensors, this would not be necessary, because in this function we seek gradients wrt to *parameters* of the model, and `inputs` refers to inputs to the model. Furthermore, since `inputs` is not a tuple of tensors, it is not actually possible to call `apply_gradient_requirements` on `inputs`. Also see discussion at https://www.internalfb.com/diff/D41687791 (pytorch@288cd3a6754d85cbcb0ce74784aa014876284b6c)?dst_version_fbid=1539905103125262&transaction_fbid=587574556076948 In summary, because `inputs` is of type `Tuple[Any,...]` and not `Tuple[Tensor,...]`, it does not make sense to call `clone` on the elements of `inputs`, nor call `apply_gradient_requirements(inputs)`. And furthermore, we do not actually need to call the latter, because `inputs` is not parameters. Thus, this diff simply removes those 2 calls. Differential Revision: D43135296 fbshipit-source-id: 30c9f20cd574920c4ed0e37903da517d47c1669d
|
This pull request was exported from Phabricator. Differential Revision: D43135296 |
|
Hi @99warriors! Thank you for your pull request. We require contributors to sign our Contributor License Agreement, and yours needs attention. You currently have a record in our system, but the CLA is no longer valid, and will need to be resubmitted. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
Summary:
In https://www.internalfb.com/code/fbsource/[76c57d350097]/fbcode/pytorch/captum/captum/_utils/gradient.py?lines=840-841 we are calling
cloneon the elements ofinputs. However, sinceinputsis of typeTuple[Any,...], those elements may not be Tensors, and thus may not have the clone method.My understanding is that we call
clonebecause inapply_gradient_requirements, we changerequires_grad=Truefor all elements ofinputs. Even if those elements were Tensors, this would not be necessary, because in this function we seek gradients wrt to parameters of the model, andinputsrefers to inputs to the model. Furthermore, sinceinputsis not a tuple of tensors, it is not actually possible to callapply_gradient_requirementsoninputs.Also see discussion at https://www.internalfb.com/diff/D41687791 (288cd3a6754d85cbcb0ce74784aa014876284b6c)?dst_version_fbid=1539905103125262&transaction_fbid=587574556076948
In summary, because
inputsis of typeTuple[Any,...]and notTuple[Tensor,...], it does not make sense to callcloneon the elements ofinputs, nor callapply_gradient_requirements(inputs). And furthermore, we do not actually need to call the latter, becauseinputsis not parameters. Thus, this diff simply removes those 2 calls.Differential Revision: D43135296