fix(asr): pad random_segment with an integer sample count - #16105
Open
udsy19 wants to merge 1 commit into
Open
Conversation
`RandomSegmentPerturbation.perturb` computed `pad_size` as
`duration_sec * sample_rate - num_samples`. `duration_sec` is a float
(the constructor default is `32.0`), so `pad_size` was a float and
`AudioSegment.pad` forwarded it to `numpy.pad`, which rejects a
non-integral `pad_width`:
TypeError: `pad_width` must be of integral type.
Every utterance shorter than `duration_sec` therefore crashed training
whenever `pad_to_duration` was enabled, unless `duration_sec` happened to
be written as an integer in the YAML config.
Round up rather than to nearest: `perturb` immediately calls
`subsegment(0.0, duration_sec)`, and `subsegment` raises `ValueError` when
`end_time` exceeds the segment duration. Rounding to nearest can leave the
padded segment marginally shorter than `duration_sec` (for example
`duration_sec=0.35` at 22050 Hz), which would swap the `TypeError` for a
`ValueError`. `math.ceil` keeps the segment at least `duration_sec` long;
`subsegment` then trims it to exactly `round(duration_sec * sample_rate)`
samples, matching the non-padding branch.
Also add `pad_to_duration` to the `random_segment` snippet in the SSL
configuration docs. The surrounding text states that "samples below the
provided segment length will be padded", but the snippet omitted the flag
and it defaults to `False`, so the documented recipe silently did not pad.
Signed-off-by: Udaya Tejas <udayatejas2004@gmail.com>
This file contains hidden or 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
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 ?
Fixes
RandomSegmentPerturbation(random_segmentaugmentation) crashing withTypeError:pad_widthmust be of integral type.whenever it has to pad a short utterance, andcorrects the SSL configuration doc that documents the padding behaviour but omits the flag that
enables it.
Collection: ASR
Changelog
nemo/collections/asr/parts/preprocessing/perturb.py: computepad_sizeas an integer numberof samples with
math.ceil, soAudioSegment.pad->numpy.padreceives an integralpad_width. Previouslyduration_sec * sample_rate - num_sampleswas a float wheneverduration_secwas a float — including the constructor default32.0— and every shortutterance raised
TypeError.math.ceilrather thanround/truncation is deliberate: thenext statement is
subsegment(0.0, duration_sec), which raisesValueErrorif the segmentends up shorter than
duration_sec, so the padded length must not round down.subsegmentthen trims to exactly
round(duration_sec * sample_rate)samples, matching the non-paddingbranch.
docs/source/asr/ssl/configs.rst: addpad_to_duration: trueto therandom_segmentsnippet.The surrounding text says "samples below the provided segment length will be padded", but the
snippet omitted the flag and it defaults to
False, so the documented recipe silently did notpad — which defeats its stated purpose of keeping durations uniform inside an SSL batch.
tests/collections/asr/test_preprocessing_segment.py: addTestRandomSegmentPerturbationcovering int and float
duration_sec, the zero-fill placement, sample rates whereduration_sec * sample_rateis not a whole number (22050 Hz), thepad_to_duration=Falsedefault, and the non-padding branch. There was previously no test for this perturbation.
Usage
Equivalently, from a training config:
GitHub Actions CI
Checks run locally (macOS, CPU, Python 3.10.18, torch 2.12.0, numpy 2.2.6):
pre-commit run --from-ref main --to-ref HEAD— all hooks pass (isort, black 24.10.0,check-case-conflict, detect-private-key, check-added-large-files).
pytest tests/collections/asr/test_preprocessing_segment.py -m "not pleasefixme"— 73 passed.pytest tests/collections/asr -m "not pleasefixme"— 1402 passed, 402 skipped, 26 failed,12 errors. All 38 failures/errors are pre-existing on
mainand environmental on a CPU-onlymacOS box (
*_gpuparametrisations, ONNX/TorchScript export, model-download errors); none isin preprocessing or perturbation code. Verified by re-running the 7 affected files with
perturb.pyreverted tomain: the failure sets are byte-identical, so this changeintroduces no new failures.
perturb.pyreverted tomainand the new tests in place, 6 of the 10new cases fail (
TypeError:pad_widthmust be of integral type.); the 4 that pass are theint-
duration_sec,pad_to_duration=Falseand long-audio controls. All 10 pass with the fix.git diff --check— clean... code-block:: yamland adds no directive, role or reference; the snippet was parsed withyaml.safe_loadto confirm it is still valid.Before your PR is "Ready for review"
Pre checks:
mathis stdlib; no new dependency and no optional import.PR Type:
Additional Information
user-visible failure: on
mainthe documentedrandom_segmentrecipe does not pad, and thedoc line would be actively misleading — it would walk readers into the
TypeError— if landedwithout the code fix.