Skip to content

Conversation

samanklesaria
Copy link
Collaborator

@samanklesaria samanklesaria commented Aug 5, 2025

This PR ports the forced_align code to the stable ABI. It uses the temporary Accessor class defined in the stable_accessor PR.

@samanklesaria samanklesaria requested a review from a team as a code owner August 5, 2025 19:22
Copy link

pytorch-bot bot commented Aug 5, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/audio/4022

Note: Links to docs will display an error until the docs builds have been completed.

❌ 21 New Failures

As of commit 93c6d68 with merge base bdd9c72 (image):

NEW FAILURES - The following jobs have failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla bot added the CLA Signed label Aug 5, 2025
@samanklesaria samanklesaria marked this pull request as draft August 6, 2025 21:31
@samanklesaria
Copy link
Collaborator Author

Note: this commit now depends on the dtype and is_cpu methods of stable::Tensor in pytorch, which have yet to be merged in. I will keep this PR in draft state until they make their way to nightly.

@samanklesaria samanklesaria changed the title Remove Accessor use in forced_align [STABLE FORCED_ALIGN ABI PORT] Remove Accessor use in forced_align Aug 7, 2025
@samanklesaria
Copy link
Collaborator Author

This PR passes all tests for forced_align on CPU except expected failures. This is because the correct handling of the expected failures requires porting the amax and item ops to the stable ABI. I will port these ops next.

@MahmoudAshraf97
Copy link

Hello, is there a chance to get any features or improvements from #3787 merged? on the other side, I managed to remove the torch dependency completely from the FA api, so I guess it's doable, only the input log_probs array needs to be a torch tensor, all other arrays can be vectors or regular arrays, check https://github.com/MahmoudAshraf97/ctc-forced-aligner/blob/main/ctc_forced_aligner/forced_align_impl.cpp for reference

@NicolasHug
Copy link
Member

Hi @MahmoudAshraf97 , we're currently trying to migrate these ops to be ABI-stable, which is our current top priority. Unfortunately, we won't have the bandwidth to consider improvements like in #3787 at the moment, sorry. It's possible that we'll be able to re-consider in the future, but the near-term goal is for torchaudio to be stable while minimizing maintenance requirements.

@samanklesaria samanklesaria changed the title [STABLE FORCED_ALIGN ABI PORT] Remove Accessor use in forced_align [STABLE ABI] Stable forced_align on cpu Aug 19, 2025
@samanklesaria samanklesaria changed the base branch from main to stable_accessor August 19, 2025 20:01
@samanklesaria samanklesaria marked this pull request as ready for review August 20, 2025 19:43
@samanklesaria
Copy link
Collaborator Author

The CI used to run fine. A recent change to the stable ABI now leads to stable::Tensor::scalar_type() being defined twice, which gives all the errors. Perhaps there's a new preferred way of importing the stable API headers?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants