Repository navigation
Add ADV audio processing dependencies #1574
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e9a56c6
6764266
7970ed5
d298119
8176af8
2c24668
10a24a2
562c70b
8583ba6
346a81d
84182d3
3ef40ac
b614a10
a1c0b8e
31d4225
7d566ce
7219e3a
2da2f6e
4efc912
1758dcc
c313422
47c1dbd
5d6f6d8
bf2ce78
332e2c7
ebb4187
5ec3af4
28cd599
23a359b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -95,13 +95,23 @@ deduplication_cuda12 = [ | |||||
| "rapidsmpf-cu12==25.10.*", | ||||||
| ] | ||||||
|
|
||||||
| audio_cpu = [ | ||||||
| audio_common = [ | ||||||
| "nemo_toolkit[asr]>=2.7.2", | ||||||
| "soundfile>=0.12.0", | ||||||
| "torchaudio", | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Consider adding a version constraint that mirrors what
Suggested change
(replace the version range with whatever NeMo 2.4.0 expects) |
||||||
| "onnx>=1.19.0", | ||||||
| "silero-vad", | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Consider pinning or bounding the version to match the API your code relies on, e.g.:
Suggested change
|
||||||
| ] | ||||||
|
Comment on lines
+98
to
+104
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If # Internal base group — do not install directly; use audio_cpu or audio_cuda12
audio_common = [
...
]This prevents accidental misuse without requiring a structural change. |
||||||
| audio_cpu = [ | ||||||
| "nemo_curator[audio_common]", | ||||||
| "onnxruntime>=1.20.1,<1.24", | ||||||
| ] | ||||||
| audio_cuda12 = [ | ||||||
| "nemo_curator[audio_cpu]", | ||||||
| "nemo_curator[audio_common]", | ||||||
| "nemo_curator[cuda12]", | ||||||
| "nvidia-cudnn-cu12", | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Please add a version pin or at minimum an upper bound (e.g.
Suggested change
|
||||||
| "onnxruntime-gpu>=1.20.1,<1.24", | ||||||
| "torchcodec", | ||||||
| ] | ||||||
|
Comment on lines
+105
to
115
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The rest of this file's CUDA extras avoid this by having the CUDA variant include the CPU variant (e.g. Consider adding a conflicts declaration: [tool.uv.conflicts]
audio = [
{ extra = "audio_cpu" },
{ extra = "audio_cuda12" },
]This gives uv the information it needs to raise an error if a user tries to resolve both extras simultaneously, rather than silently producing a broken environment. |
||||||
|
|
||||||
| image_cpu = [ | ||||||
|
|
@@ -211,7 +221,7 @@ all = [ | |||||
| ] | ||||||
|
|
||||||
| [dependency-groups] | ||||||
| build = ["setuptools", "torch<=2.9.1"] | ||||||
| build = ["setuptools", "torch<=2.9.1", "Cython", "packaging"] | ||||||
| dev = ["jupyter"] | ||||||
| linting = ["pre-commit", "ruff==0.14.10"] | ||||||
| test = [ | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why make a distinct group for audio common? Audio cuda inherits from audio cpu
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
audio_common exists because audio_cpu and audio_cuda12 need different onnxruntime packages (onnxruntime vs onnxruntime-gpu). If audio_cuda12 inherited from audio_cpu, both CPU and GPU versions of onnxruntime would be installed, which can cause conflicts. audio_common shares everything except the onnxruntime variant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha!