Fix GreedyBatchedCTCInfer regression from GreedyCTCInfer. #9347
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
decoder_lengths is allowed to be on CPU even when decoder_output is on GPU. This matches the behavior of GreedyCTCInfer. Even though that behavior is unintentional, there is code depending on that behavior, including our jupyter notebooks. Signed-off-by: Daniel Galvez <dgalvez@nvidia.com>
titu1994
previously approved these changes
May 30, 2024
Signed-off-by: titu1994 <titu1994@users.noreply.github.com>
titu1994
approved these changes
May 30, 2024
|
|
titu1994
pushed a commit
that referenced
this pull request
May 30, 2024
* Fix GreedyBatchedCTCInfer regression from GreedyCTCInfer. decoder_lengths is allowed to be on CPU even when decoder_output is on GPU. This matches the behavior of GreedyCTCInfer. Even though that behavior is unintentional, there is code depending on that behavior, including our jupyter notebooks. Signed-off-by: Daniel Galvez <dgalvez@nvidia.com> * Apply isort and black reformatting Signed-off-by: titu1994 <titu1994@users.noreply.github.com> --------- Signed-off-by: Daniel Galvez <dgalvez@nvidia.com> Signed-off-by: titu1994 <titu1994@users.noreply.github.com> Co-authored-by: Somshubra Majumdar <titu1994@gmail.com> Co-authored-by: titu1994 <titu1994@users.noreply.github.com> Co-authored-by: Nithin Rao <nithinrao.koluguri@gmail.com> (cherry picked from commit aed9d07)
8 tasks
titu1994
added a commit
that referenced
this pull request
May 30, 2024
)" This reverts commit aed9d07.
ericharper
pushed a commit
that referenced
this pull request
Jun 4, 2024
) * Fix GreedyBatchedCTCInfer regression from GreedyCTCInfer. (#9347) * Fix GreedyBatchedCTCInfer regression from GreedyCTCInfer. decoder_lengths is allowed to be on CPU even when decoder_output is on GPU. This matches the behavior of GreedyCTCInfer. Even though that behavior is unintentional, there is code depending on that behavior, including our jupyter notebooks. Signed-off-by: Daniel Galvez <dgalvez@nvidia.com> * Apply isort and black reformatting Signed-off-by: titu1994 <titu1994@users.noreply.github.com> --------- Signed-off-by: Daniel Galvez <dgalvez@nvidia.com> Signed-off-by: titu1994 <titu1994@users.noreply.github.com> Co-authored-by: Somshubra Majumdar <titu1994@gmail.com> Co-authored-by: titu1994 <titu1994@users.noreply.github.com> Co-authored-by: Nithin Rao <nithinrao.koluguri@gmail.com> (cherry picked from commit aed9d07) * Add Packaging to install documentation Signed-off-by: smajumdar <titu1994@gmail.com> * Mark confidence tests as please fix me Signed-off-by: smajumdar <titu1994@gmail.com> --------- Signed-off-by: smajumdar <titu1994@gmail.com> Co-authored-by: Daniel Galvez <galv@users.noreply.github.com> Co-authored-by: Pablo Garay <palenq@gmail.com>
github-actions bot
pushed a commit
that referenced
this pull request
Jun 4, 2024
) * Fix GreedyBatchedCTCInfer regression from GreedyCTCInfer. (#9347) * Fix GreedyBatchedCTCInfer regression from GreedyCTCInfer. decoder_lengths is allowed to be on CPU even when decoder_output is on GPU. This matches the behavior of GreedyCTCInfer. Even though that behavior is unintentional, there is code depending on that behavior, including our jupyter notebooks. Signed-off-by: Daniel Galvez <dgalvez@nvidia.com> * Apply isort and black reformatting Signed-off-by: titu1994 <titu1994@users.noreply.github.com> --------- Signed-off-by: Daniel Galvez <dgalvez@nvidia.com> Signed-off-by: titu1994 <titu1994@users.noreply.github.com> Co-authored-by: Somshubra Majumdar <titu1994@gmail.com> Co-authored-by: titu1994 <titu1994@users.noreply.github.com> Co-authored-by: Nithin Rao <nithinrao.koluguri@gmail.com> (cherry picked from commit aed9d07) * Add Packaging to install documentation Signed-off-by: smajumdar <titu1994@gmail.com> * Mark confidence tests as please fix me Signed-off-by: smajumdar <titu1994@gmail.com> --------- Signed-off-by: smajumdar <titu1994@gmail.com> Co-authored-by: Daniel Galvez <galv@users.noreply.github.com> Co-authored-by: Pablo Garay <palenq@gmail.com>
8 tasks
titu1994
added a commit
that referenced
this pull request
Jun 4, 2024
) (#9371) * Fix GreedyBatchedCTCInfer regression from GreedyCTCInfer. (#9347) * Fix GreedyBatchedCTCInfer regression from GreedyCTCInfer. decoder_lengths is allowed to be on CPU even when decoder_output is on GPU. This matches the behavior of GreedyCTCInfer. Even though that behavior is unintentional, there is code depending on that behavior, including our jupyter notebooks. * Apply isort and black reformatting --------- (cherry picked from commit aed9d07) * Add Packaging to install documentation * Mark confidence tests as please fix me --------- Signed-off-by: smajumdar <titu1994@gmail.com> Co-authored-by: Somshubra Majumdar <titu1994@gmail.com> Co-authored-by: Daniel Galvez <galv@users.noreply.github.com> Co-authored-by: Pablo Garay <palenq@gmail.com>
BoxiangW
pushed a commit
to BoxiangW/NeMo
that referenced
this pull request
Jun 5, 2024
* Fix GreedyBatchedCTCInfer regression from GreedyCTCInfer. decoder_lengths is allowed to be on CPU even when decoder_output is on GPU. This matches the behavior of GreedyCTCInfer. Even though that behavior is unintentional, there is code depending on that behavior, including our jupyter notebooks. Signed-off-by: Daniel Galvez <dgalvez@nvidia.com> * Apply isort and black reformatting Signed-off-by: titu1994 <titu1994@users.noreply.github.com> --------- Signed-off-by: Daniel Galvez <dgalvez@nvidia.com> Signed-off-by: titu1994 <titu1994@users.noreply.github.com> Co-authored-by: Somshubra Majumdar <titu1994@gmail.com> Co-authored-by: titu1994 <titu1994@users.noreply.github.com> Co-authored-by: Nithin Rao <nithinrao.koluguri@gmail.com> Signed-off-by: Boxiang Wang <boxiangw@nvidia.com>
BoxiangW
pushed a commit
to BoxiangW/NeMo
that referenced
this pull request
Jun 5, 2024
…IDIA#9347)" (NVIDIA#9351) This reverts commit aed9d07. Signed-off-by: Boxiang Wang <boxiangw@nvidia.com>
BoxiangW
pushed a commit
to BoxiangW/NeMo
that referenced
this pull request
Jun 5, 2024
… (NVIDIA#9350) (NVIDIA#9371) * Fix GreedyBatchedCTCInfer regression from GreedyCTCInfer. (NVIDIA#9347) * Fix GreedyBatchedCTCInfer regression from GreedyCTCInfer. decoder_lengths is allowed to be on CPU even when decoder_output is on GPU. This matches the behavior of GreedyCTCInfer. Even though that behavior is unintentional, there is code depending on that behavior, including our jupyter notebooks. * Apply isort and black reformatting --------- (cherry picked from commit aed9d07) * Add Packaging to install documentation * Mark confidence tests as please fix me --------- Signed-off-by: smajumdar <titu1994@gmail.com> Co-authored-by: Somshubra Majumdar <titu1994@gmail.com> Co-authored-by: Daniel Galvez <galv@users.noreply.github.com> Co-authored-by: Pablo Garay <palenq@gmail.com> Signed-off-by: Boxiang Wang <boxiangw@nvidia.com>
janekl
pushed a commit
that referenced
this pull request
Jun 12, 2024
* Fix GreedyBatchedCTCInfer regression from GreedyCTCInfer. decoder_lengths is allowed to be on CPU even when decoder_output is on GPU. This matches the behavior of GreedyCTCInfer. Even though that behavior is unintentional, there is code depending on that behavior, including our jupyter notebooks. Signed-off-by: Daniel Galvez <dgalvez@nvidia.com> * Apply isort and black reformatting Signed-off-by: titu1994 <titu1994@users.noreply.github.com> --------- Signed-off-by: Daniel Galvez <dgalvez@nvidia.com> Signed-off-by: titu1994 <titu1994@users.noreply.github.com> Co-authored-by: Somshubra Majumdar <titu1994@gmail.com> Co-authored-by: titu1994 <titu1994@users.noreply.github.com> Co-authored-by: Nithin Rao <nithinrao.koluguri@gmail.com> Signed-off-by: Jan Lasek <janek.lasek@gmail.com>
janekl
pushed a commit
that referenced
this pull request
Jun 12, 2024
) (#9371) * Fix GreedyBatchedCTCInfer regression from GreedyCTCInfer. (#9347) * Fix GreedyBatchedCTCInfer regression from GreedyCTCInfer. decoder_lengths is allowed to be on CPU even when decoder_output is on GPU. This matches the behavior of GreedyCTCInfer. Even though that behavior is unintentional, there is code depending on that behavior, including our jupyter notebooks. * Apply isort and black reformatting --------- (cherry picked from commit aed9d07) * Add Packaging to install documentation * Mark confidence tests as please fix me --------- Signed-off-by: smajumdar <titu1994@gmail.com> Co-authored-by: Somshubra Majumdar <titu1994@gmail.com> Co-authored-by: Daniel Galvez <galv@users.noreply.github.com> Co-authored-by: Pablo Garay <palenq@gmail.com> Signed-off-by: Jan Lasek <janek.lasek@gmail.com>
rohitrango
pushed a commit
to rohitrango/NeMo
that referenced
this pull request
Jun 25, 2024
* Fix GreedyBatchedCTCInfer regression from GreedyCTCInfer. decoder_lengths is allowed to be on CPU even when decoder_output is on GPU. This matches the behavior of GreedyCTCInfer. Even though that behavior is unintentional, there is code depending on that behavior, including our jupyter notebooks. Signed-off-by: Daniel Galvez <dgalvez@nvidia.com> * Apply isort and black reformatting Signed-off-by: titu1994 <titu1994@users.noreply.github.com> --------- Signed-off-by: Daniel Galvez <dgalvez@nvidia.com> Signed-off-by: titu1994 <titu1994@users.noreply.github.com> Co-authored-by: Somshubra Majumdar <titu1994@gmail.com> Co-authored-by: titu1994 <titu1994@users.noreply.github.com> Co-authored-by: Nithin Rao <nithinrao.koluguri@gmail.com>
rohitrango
pushed a commit
to rohitrango/NeMo
that referenced
this pull request
Jun 25, 2024
…IDIA#9347)" (NVIDIA#9351) This reverts commit db26475.
rohitrango
pushed a commit
to rohitrango/NeMo
that referenced
this pull request
Jun 25, 2024
… (NVIDIA#9350) (NVIDIA#9371) * Fix GreedyBatchedCTCInfer regression from GreedyCTCInfer. (NVIDIA#9347) * Fix GreedyBatchedCTCInfer regression from GreedyCTCInfer. decoder_lengths is allowed to be on CPU even when decoder_output is on GPU. This matches the behavior of GreedyCTCInfer. Even though that behavior is unintentional, there is code depending on that behavior, including our jupyter notebooks. * Apply isort and black reformatting --------- (cherry picked from commit db26475) * Add Packaging to install documentation * Mark confidence tests as please fix me --------- Signed-off-by: smajumdar <titu1994@gmail.com> Co-authored-by: Somshubra Majumdar <titu1994@gmail.com> Co-authored-by: Daniel Galvez <galv@users.noreply.github.com> Co-authored-by: Pablo Garay <palenq@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do ?
decoder_lengths is allowed to be on CPU even when decoder_output is on GPU. This matches the behavior of GreedyCTCInfer. Even though that behavior is unintentional, there is code depending on that behavior, including our jupyter notebooks.
Testing
I tested in the following way. I ran:
CUDA_VISIBLE_DEVICES="" ensures that the test runs when on CPU-only machines, as well as on machines with GPU. My previous PR related to this failed to test on GPU, which caused the crash we see today when we run with lengths on CPU but logprobs on GPU.
I also manually ran examples/notebooks/asr/ASR_with_Subword_Tokenization.ipynb and examples/notebooks/asr/ASR_with_NeMo.ipynb manually. These run to completion. I had to comment out the part that install NeMo from pypi, which would overwrite my local installation.
Collection: ASR
PR Type:
If you haven't finished some of the above items you can still open "Draft" PR.
Who can review?
Anyone in the community is free to review the PR once the checks have passed.
Contributor guidelines contains specific people who can review PRs to various areas.
Additional Information