Repository navigation
audio: remove conv1d and depthwise_conv1d; callers use Graph::conv1d - #122
Merged
Merged
Conversation
Every backend's conv1d operator takes a group count now (CUTLASS runs the depthwise case as its own kernel), so the two compositions that stood in for it are gone. Their 27 call sites call g.conv1d directly. Functions that only took dtype and device to pass to them drop both and return Value. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.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.
Removes
audio::conv1dandaudio::depthwise_conv1d. All 27 call sites (s2mel, campplus, emotion, w2v_bert, semantic_codec, features,layers::Conv1d, andstft/resample/downsample1dinaudio.rs) now callGraph::conv1ddirectly, and the three depthwise ones passgroups = channels.Why
Both were compositions that worked around CUDA's
conv2d, which rejects any group count above one. The nativeconv1doperator now takes groups on CPU, CUDA, Metal and Vulkan, and CUTLASS has its own kernel for the depthwise case. cosyvoice3's FSMN already used it.API changes
The composition needed
dtype/deviceto build its padding, and without it these functions never fail. So they drop both parameters and returnValueinstead ofResult<Value>:layers::Conv1d::graph,emotion::encoder(and its privateconv_moduleandencoder_layer),semantic_codec::decode(and its privatevocos_backboneandconvnext_block), ands2mel::wavenet. w2v_bert'sconv_modulekeeps them because its causalpad1dstill needs them.Tests
./build/unittestandcargo testpass.tests/audio.rs: removed the test of the composition against the naive loop. The test ofGraph::conv1dagainst the same loop stays. Also removed the depthwise-vs-grouped test.tests/audio_cuda.rs: moved the grouped and depthwise cases (including 31 taps) intoconv1d_on_the_card_is_conv1d_on_the_hostand dropped the separate depthwise test.audio_cuda,indextts,indextts_gpt,cosyvoice3). NVML on the dev machine reported a driver/library mismatch.🤖 Generated with Claude Code