Conversation
yqwangustc
left a comment
There was a problem hiding this comment.
Changes look good to me. Only some minor comment on docstring for the "feature_stacking" vs "stacking", and "nemo_transformer_audio_causal_mode==checkpoint" means.
If "feature_stacking" is very similar to "stacking", and the only difference is how they treat padding when doing the stacking, maybe consider merge them to one mode ?
| return num_frames // self.encoder_time_stride | ||
|
|
||
| if self.pre_encode == "stacking": | ||
| if self.pre_encode in ("stacking", "feature_stacking"): |
There was a problem hiding this comment.
Do we have any docstring to explain what's the difference between "stacking" and "feature_stacking" ?
There was a problem hiding this comment.
Removed "feature_stacking" in 15fe07e. Now both legacy and rope encoders use "stacking" mode, just with different stacking_pad_mode.
|
|
||
| raise ValueError( | ||
| f"Unsupported Nemo TransformerEncoder pre_encode={self.pre_encode!r}; " | ||
| "expected 'conv', 'depth_conv', or 'stacking'." |
There was a problem hiding this comment.
looks like here should be "expected 'conv', 'depth_conv', 'stacking' or 'feature_stacking' ?
| left_context = getattr(args, "nemo_transformer_audio_left_context", None) | ||
| if left_context is not None: | ||
| encoder_cfg.left_context = None if left_context < 0 else left_context | ||
| causal_mode = getattr(args, "nemo_transformer_audio_causal_mode", "checkpoint") |
There was a problem hiding this comment.
Do we have a docstring to explain what "nemo_transformer_audio_causal_mode=checkpoint" mean ?
|
@yqwangustc not sure why some checks are indefinitely pending, do you know? |
|
/ok to test 8100f03 |
| [[tool.uv.dependency-metadata]] | ||
| name = "transformer-engine" | ||
| version = "2.18.0+27486e03" | ||
| version = "2.20.0.dev0+7ba2f9f1" |
There was a problem hiding this comment.
The new TE version is required which enables thd_attention_policies. See NVIDIA/TransformerEngine#3274
There was a problem hiding this comment.
Looks like this change has landed. Are we safe to make this pyproject.toml file changes now ?
There was a problem hiding this comment.
Yeah it should be safe, but we need code-owner approval.
7c989cf to
cd3c099
Compare
|
/ok to test cd3c099 |
Port the dual-mode causal/offline audio encoder onto the current GitHub audio model interfaces. Preserve one encoder parameterization across modes and use Transformer Engine's grouped THD attention-policy dispatch for mixed packed batches. Signed-off-by: Desh Raj <deshr@nvidia.com>
Use one stacking pre-encoder for legacy and RoPE audio towers, with an explicit learned-or-zero padding policy. Preserve both checkpoint layouts and normalize the former feature_stacking configuration as a compatibility alias. Signed-off-by: Desh Raj <deshr@nvidia.com>
Signed-off-by: Desh Raj <deshr@nvidia.com>
cd3c099 to
e22de2a
Compare
@desh2608, there was an error processing your request: See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/1/ |
|
/ok to test e22de2a |
|
/ok to test 0af3991 |
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/37052597572 |
What does this PR do?
Adds an opt-in dual-mode RoPE audio encoder that uses one shared encoder parameterization for offline and causal audio processing.
The implementation:
pre_encode="stacking"implementation with configurable learned or zero padding;architecture=rope_transformerselection for the new path.TransformerEngine dependency
Dual-mode training requires the latest TransformerEngine with the grouped mixed-THD attention-policy API from NVIDIA/TransformerEngine#3274. The mixed packed path passes
thd_attention_policieswiththd_attention_policy_dispatch="grouped"; TransformerEngine versions that predate that API are not sufficient.Configuration and compatibility
self_attention_model: ropeare recognized asrope_transformercheckpoints.feat_inmaps to the encodern_melssetting.pre_encode="stacking"when frame stacking is selected.stacking_pad_mode="learned"preserves the legacy NGPT padding behavior;stacking_pad_mode="zeros"matches the RoPE NeMo encoder.feature_stackingvalue remains accepted as a compatibility alias and normalizes to zero-paddedstacking.proj_out.weightpluspad_frame, while zero padding usesproj.weightwithout a trainable padding parameter.--nemo-transformer-audio-causal-modesupportscheckpoint,causal, andofflineruntime selection.Issue tracking
Linked issue: N/A — internal feature port.
Validation
Focused unit tests:
Result: 40 passed, 6 skipped. The skipped cases require CUDA and Transformer Engine; the suite includes a real-TE CUDA parity test for mixed windowed packed attention.
The stacking tests cover learned and zero padding on dense and already-packed inputs, padding numerics, checkpoint-key compatibility, legacy config normalization, and RoPE configuration validation.
Additional checks completed:
tools/autoformat.shin check modeisortand Black formattinggit diff --checkContribution process
Pre-checks
tools/autoformat.shon my PRThis PR changes attention/model architecture behavior and adds new tests, so it uses the Run functional tests label.