-
Notifications
You must be signed in to change notification settings - Fork 534
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
[TorchToLinalg] Implement lowering of torch.aten.rrelu_with_noise and torch.aten.rrelu_with_noise_backward ops #3645
Conversation
What's the exact issue that you have? Is it that you want the result of the forward op to be used with the test for the backward op? Edit: If that's the only concern, then you can have a forward op in your test generating the noise value which you can pass to the backward op in the test. |
Yes, that was the issue. I made a test like you proposed, but cannot seem to get close to the golden value output, and I've tried a lot of variations of tests and dimensionalities for input and noise, as well as playing around with implementation itself, to no avail. I assume because of randomized noise it makes it hard to match between two ops, but I'm really not sure where lies the issue: in my implementation or in the way test is formed. Here is test and the output: torch-mlir/projects/pt1/python/torch_mlir_e2e_test/test_suite/backprop.py Lines 453 to 488 in e70be44
|
Are you sure that the decomposition you've added is functionally/numerically correct? If not, then what you can do is run a PyTorch code having the same decomposition which you added in this PR, and compare its result with the PyTorch op result. |
210e5e8
to
9570b7b
Compare
Hi @bosko-syrmia, some of the tests are passing and you need to update them in the xfail_set. |
Hi Vivek, it's because I hadn't updated my fork. In the process, I messed up my branch so I will probably cherry-pick this work onto a different branch and create new PR and link it here #3748 |
Hi @bosko-syrmia, I think you can close this PR now. |
I'm not sure if e2e tests that test each of the ops individually are sufficient, since (to my understanding)
rrelu_with_noise
generatesnoise
tensor in forward, butrrelu_with_noise_backward
uses that same tensor to multiplygradOutput
as the return value in backprop. Is there a better way to test it with, say simple NN written with Pytorch that is then compiled using the MLIR infra and tested against original Pytorch output. Would that even be necessary?Also, I had to remove some tests for
AtenRreluOp
fromFX_IMPORTER_XFAIL_SET
because they no longer failed after lowering oftorch.aten.rrelu_with_noise
. I suppose it has something to do with the way ATen implements rrelu by calling rrelu_with_noise with empty tensor: