-
Notifications
You must be signed in to change notification settings - Fork 739
[STABLE ABI] Port forced_align #4079
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
base: main
Are you sure you want to change the base?
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/audio/4079
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Active SEVsThere are 1 currently active SEVs. If your PR is affected, please view them below: ✅ No FailuresAs of commit b308882 with merge base 3b0e7a6 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
b3a5a0e to
282bd7f
Compare
samanklesaria
left a comment
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.
Looks good to me!
007452e to
bae56dd
Compare
bae56dd to
b308882
Compare
NicolasHug
left a comment
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.
Thanks a lot for the PR @pearu .
Agreed to move forward with this, and let's try to upstream the components in libtorchaudio/stable as much as possible.
| STD_TORCH_CHECK( | ||
| targets.size(1) == torchaudio::util::max<index_t>(targetLengths), | ||
| "target length mismatch"); | ||
| }); |
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.
On the above, can you help me understand why we need to use STABLE_DISPATCH_INDEX_TYPES now? Basically, which part of at::max(inputLengths).item().toInt() do we need to work-around? Is this a permanent workaround, or something we'll be able to simplify in the future when more APIs are ported to the stable part?
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.
Currently, targetLengths can be a tensor with int32 or int64 items. If one uses, say, torchaudio::util::max<int32_t>(targetLengths) when targetLengths dtype is int64, the result of max may be wrong. The usage of STABLE_DISPATCH_INDEX_TYPES makes calling max safe.
We can simplify this in future after we are able to rewrite at::max(inputLengths).item().toInt() using stable ABI methods.
As in the title.
This PR is marked as Draft as it requires the torch PR stack starting at pytorch/pytorch#161891 (read: CI failures are expected).
Same as #4022 but using C++ torch::stable::Tensor API instead of C shim Tensor API and this PR includes also the GPU port.
PLEASE NOTE THAT THE TORCHAUDIO REPOSITORY IS NO LONGER ACTIVELY MONITORED. You may not get a response. For open discussions, visit https://discuss.pytorch.org/.