Add cumprod support for device mps #104688
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/104688
Note: Links to docs will display an error until the docs builds have been completed. ✅ 1 Unrelated FailureAs of commit cca389b: UNSTABLE - The following job failed but was likely due to flakiness present on trunk and has been marked as unstable:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
pytorch-bot
bot
added
ciflow/mps
| } | ||
| auto input = dtype.has_value() ? self.to(dtype.value()) : self; | ||
|
|
||
| // issue #103810551: cumprod is horribly broken for int8, int16 and as chances for overflow is pretty high, cast to |
There was a problem hiding this comment.
I'm not sure what issue #103810551 is and where I want to check if this applies to cumprod as well as cumsum. So far I assumed it did but seeing the actual issue in whatever tracker it lies would help. If it didn't I could remove a decent chunk of this function. If it does not I can dedupe this with cumsum and make a general helper function
There was a problem hiding this comment.
from the work done on kulinseth#419, it appears constraint does hold for both functions so I will merge them into a single one as is done in this pr
| @@ -393,7 +394,7 @@ Tensor logit_mps(const Tensor& self, c10::optional<double> eps) { | |||
|
|
|||
| mps::unary_op(input, | |||
| result, | |||
| "cumsum_out_mp" + std::to_string(dim), | |||
| "cumsum_out_mps" + std::to_string(dim), | |||
|
@kulinseth it appears this might be a duplicate pr with something on your branch: kulinseth#419, happy to close my pr out if need be |
|
I'm not sure why macos-12-py3 tests are failing for |
bdhirsh
added
the
triaged
|
@kulinseth just wanted to bump here. Also @DenisVieriu97 if you are also cleared to review these prs |
Mr4k
force-pushed
the
peter/cumulativeops
branch
from
ef75892
to
5311353
Compare
Mr4k
force-pushed
the
peter/cumulativeops
branch
from
6fabad5
to
f2cd347
Compare
…'t hit the same edge case as cumprod
|
@kulinseth @DenisVieriu97 I finally triaged all the bugs working on this pr uncovered and it's now ready to go! Edit: Sorry for "@"ing you both, I didn't know I had permission to merge without intervention from one of you. |
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Merge failedReason: New commits were pushed while merging. Please rerun the merge command. Details for Dev Infra teamRaised by workflow job |
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Related to #77764
Add support for the cumprod operation (which in turn allows its gradient). This also allows us to compute the gradient of prod since it was blocked behind cumprod in the case where exactly one element of the tensor was 0.