fix(paraformer): trim timestamp predictor padding per sample - #3554
Conversation
LauraGPT
left a comment
There was a problem hiding this comment.
I reproduced the focused result (5 passed), but the two timestamp PRs are not currently green together. Applying #3553 then #3554 and running python -m pytest -q tests/test_paraformer_timestamp_contract.py tests/test_paraformer_timestamp_padding.py tests/test_punc_model_none.py gives 20 passed, 1 failed: #3553 constructs a lightweight Paraformer without predictor, while the new #3554 branch reads self.predictor directly and raises AttributeError. Please preserve the intended production behavior while making the access compatible, e.g. bind predictor = getattr(self, "predictor", None) before checking getattr(predictor, "tail_mask", None). Please add a combined regression (or extend the fixture) so both PRs remain green together. DCO is also missing on 947d7116eaffea2dde962a49d5251e02eb55eebb; amend with --signoff before the force-with-lease push.
|
Fresh current-main revalidation confirms the existing requested change without adding scope. Against
Environment: Python 3.12, NumPy 1.26.4, SciPy 1.12.0, Torch 2.11.0+cu130. The server-wide NumPy 2.x/SciPy ABI mismatch was isolated and is not attributed to this PR. Please apply the minimal compatibility change on #3554 and amend both PR commits with your own |
947d711 to
6042ace
Compare
|
Thanks. I applied the requested minimal compatibility change by binding |
LauraGPT
left a comment
There was a problem hiding this comment.
The requested compatibility fix and missing-predictor regression are correct. I applied exact heads #3553 449414665d36953f3bd83e514c31125a228c0fe4 and #3554 6042ace22d617a69c47735503cb67be04c7825fd together on current main@55b662ccf9ea77237ba9253b3bddd953d4184f84; all 24 focused timestamp, padding, punctuation-none, and timestamp-tool tests pass. py_compile and git diff --check also pass.
The only remaining blocker is the trailer syntax. The commit currently contains:
Signed-off-by: John6666 186692226+John6666cat@users.noreply.github.com
DCO requires angle brackets:
Signed-off-by: John6666 <186692226+John6666cat@users.noreply.github.com>
Please ensure user.name and user.email are configured, then run git commit --amend --no-edit --signoff and git push --force-with-lease. Please verify the resulting message with git log -1 --format=%B. No code changes are requested.
f7d8545 to
aa4375b
Compare
Signed-off-by: John6666 <186692226+John6666cat@users.noreply.github.com>
aa4375b to
b2fb22d
Compare
LauraGPT
left a comment
There was a problem hiding this comment.
Revalidated the exact cleaned replacement head b2fb22d1addcfa0284b3ca0c63bf2e02d02a6ad4 after #3553 on current main@917ca2f4a673b4ee8b0aea4b64fbe2d6c2034a67.
- the accidental C#
obj/Debugartifacts are absent; only the two intended files remain - the source tree is identical to the previously reviewed implementation
- the corrected
Signed-off-bytrailer satisfies DCO - the combined #3553 + #3554 focused suite passes 24/24
py_compileandgit diff --checkpass
Approved for merge after #3553.
|
Thanks for revalidating the cleaned replacement head and the combined suite. I appreciate the thorough review. |
Summary
Fix per-sample timestamp predictor padding handling in Paraformer dynamic batches.
When the predictor has
tail_mask=True, timestamp inputs are now trimmed per sample using:encoder_out_lens[i]tail_threshold > 0Behavior is unchanged when
tail_mask=False, when the predictor has notail_maskattribute, or when the tail threshold is zero.A CPU-only, network-free regression test is included.
Type of change
Validation
Focused regression test:
python -m pytest -q tests/test_paraformer_timestamp_padding.pyOn the unmodified baseline:
3 failed, 2 passedWith this change:
5 passedThe tests cover:
per-sample trimming with
tail_mask=Trueunchanged behavior with
tail_mask=Falseunchanged behavior when
tail_maskis absentzero
tail_thresholdclamping to both timestamp tensor widths
python -m compileall funasr examples testsDocs or links checked
Runtime/deployment command tested
Also checked:
git diff --checkUser impact
In dynamic batches, shorter samples can otherwise pass padded predictor frames into timestamp post-processing.
This change limits timestamp processing to each sample's valid encoder extent when tail masking is enabled, preventing padded tail frames from affecting generated timestamps while preserving the existing behavior for other predictor configurations.
Notes for reviewers
The scope is intentionally narrow and limited to Paraformer timestamp padding.
This is separate from the
pred_timestamp/ speaker-output contract fix handled in PR A.No EParaformer, English/BPE, or broader timestamp flag normalization changes are included.
The regression test is CPU-only and requires no network access or model download.