Skip to content
4 changes: 0 additions & 4 deletions nemo_curator/backends/experimental/ray_data/adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,6 @@ def process_dataset(self, dataset: Dataset, ignore_head_node: bool = False) -> D
Returns:
Dataset: Processed Ray Data dataset
"""
# TODO: Support nvdecs / nvencs
if self.stage.resources.gpus <= 0 and (self.stage.resources.nvdecs > 0 or self.stage.resources.nvencs > 0):
msg = "Ray Data does not support nvdecs / nvencs. Please use gpus instead."
raise ValueError(msg)

is_actor_stage_ = self.stage.ray_stage_spec().get(RayStageSpecKeys.IS_ACTOR_STAGE, is_actor_stage(self.stage))

Expand Down
9 changes: 3 additions & 6 deletions nemo_curator/backends/xenna/adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
# limitations under the License.

from cosmos_xenna.pipelines import v1 as pipelines_v1
from cosmos_xenna.ray_utils.resources import NodeInfo as XennaNodeInfo
from cosmos_xenna.ray_utils.resources import Resources as XennaResources
from cosmos_xenna.ray_utils.resources import WorkerMetadata as XennaWorkerMetadata
from cosmos_xenna.pipelines.private.resources import NodeInfo as XennaNodeInfo
from cosmos_xenna.pipelines.private.resources import Resources as XennaResources
from cosmos_xenna.pipelines.private.resources import WorkerMetadata as XennaWorkerMetadata
from loguru import logger

from nemo_curator.backends.base import BaseStageAdapter, NodeInfo, WorkerMetadata
Expand Down Expand Up @@ -44,9 +44,6 @@ def required_resources(self) -> XennaResources:
return XennaResources(
cpus=self.processing_stage.resources.cpus,
gpus=self.processing_stage.resources.gpus,
nvdecs=self.processing_stage.resources.nvdecs,
nvencs=self.processing_stage.resources.nvencs,
entire_gpu=self.processing_stage.resources.entire_gpu,
)

@property
Expand Down
6 changes: 1 addition & 5 deletions nemo_curator/pipeline/pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ def __repr__(self) -> str:
stage_info = ", ".join([f"{s.name}({s.__class__.__name__})" for s in self.stages])
return f"Pipeline(name='{self.name}', stages=[{stage_info}])"

def describe(self) -> str: # noqa: C901
def describe(self) -> str:
"""Get a detailed description of the pipeline stages and their requirements."""
lines = [
f"Pipeline: {self.name}",
Expand All @@ -148,10 +148,6 @@ def describe(self) -> str: # noqa: C901
lines.append(f" Resources: {stage.resources.cpus} CPUs")
if stage.resources.requires_gpu:
lines.append(f" GPU Memory: {stage.resources.gpu_memory_gb} GB ({stage.resources.gpus} GPUs)")
if stage.resources.nvdecs > 0:
lines.append(f" NVDEC: {stage.resources.nvdecs}")
if stage.resources.nvencs > 0:
lines.append(f" NVENC: {stage.resources.nvencs}")

lines.append(f" Batch size: {stage.batch_size}")

Expand Down
9 changes: 4 additions & 5 deletions nemo_curator/stages/resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,13 @@ class Resources:
Attributes:
cpus: Number of CPU cores required
gpu_memory_gb: GPU memory required in GB (Only for single-GPU stages)
nvdecs: Number of NVDEC units required
nvencs: Number of NVENC units required
entire_gpu: Whether to allocate entire GPU regardless of memory (This also gives you nvdecs and nvencs of that GPU)
entire_gpu: Whether to allocate entire GPU regardless of memory
gpus: Number of GPUs required (Only for multi-GPU stages)
Comment thread
abhinavg4 marked this conversation as resolved.
"""
Comment on lines 36 to 41

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Check that documentation is updated. Several doc files still reference removed nvdecs/nvencs fields:

  • docs/curate-video/tutorials/pipeline-customization/add-cust-stage.md
  • docs/curate-video/tutorials/pipeline-customization/add-cust-model.md
  • docs/curate-video/tutorials/pipeline-customization/add-cust-env.md
  • docs/curate-video/index.md
  • docs/about/concepts/video/abstractions.md

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@suiyoubi @praateekmahajan Can you take a look at this? Not addressing this in this PR.


# TODO : Revisit this gpu_memory_gb, gpus, entire_gpu too many variables for gpu
cpus: float = 1.0
gpu_memory_gb: float = 0.0
nvdecs: int = 0
nvencs: int = 0
entire_gpu: bool = False
gpus: float = 0.0

Expand All @@ -70,6 +66,9 @@ def __post_init__(self):
error_message += "Please use gpus for multi-GPU stages."
raise ValueError(error_message)

if self.entire_gpu:
self.gpus = 1.0
Comment on lines +69 to +70

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: When entire_gpu=True, self.gpus is set to 1.0 after validation checks. If user also sets gpu_memory_gb > 0, the validation on line 52 will fail before reaching this code, which is correct. However, if user sets both entire_gpu=True and gpus > 0, this assignment will silently overwrite their explicit gpus value without raising an error. Should there be a validation check to prevent setting both entire_gpu=True and gpus > 0 explicitly?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I honestly think the best is to drop entire_gpu and only provide gpu_memory_gb and gpus. @ayushdg @praateekmahajan what do you think

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with this, entire_gpu has been confusing for me the whole time it existed. I think it was there for nvenc/nvdec reasons (which I never understood well). So if we're nuking nvenc/nvdec support this should be good to go

@abhinavg4

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine with me too.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed let's just drop this

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will make a seperate issue for this to track. Let's keep it seperate. Wanna make sure we do it properly across documentation and code and stuff


@property
def requires_gpu(self) -> bool:
"""Check if this stage requires GPU resources."""
Expand Down
10 changes: 2 additions & 8 deletions nemo_curator/stages/video/clipping/clip_extraction_stages.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,12 @@
from dataclasses import dataclass
from typing import Any

from cosmos_xenna.ray_utils.resources import _get_local_gpu_info, _make_gpu_resources_from_gpu_name
from loguru import logger

from nemo_curator.backends.base import WorkerMetadata
from nemo_curator.backends.experimental.utils import RayStageSpecKeys
from nemo_curator.stages.base import ProcessingStage
from nemo_curator.stages.resources import Resources, _get_gpu_memory_gb
from nemo_curator.stages.resources import Resources
from nemo_curator.tasks.video import Clip, Video, VideoTask
from nemo_curator.utils import grouping
from nemo_curator.utils.operation_utils import make_pipeline_temporary_dir
Expand Down Expand Up @@ -79,12 +78,7 @@ def __post_init__(self) -> None:
if self.encoder == "h264_nvenc" or self.use_hwaccel:
if self.nb_streams_per_gpu > 0:
# Assume that we have same type of GPUs
gpu_info = _get_local_gpu_info()[0]
nvencs = _make_gpu_resources_from_gpu_name(gpu_info.name).num_nvencs
gpu_memory_gb = _get_gpu_memory_gb()
self.resources = Resources(
nvencs=nvencs // self.nb_streams_per_gpu, gpu_memory_gb=gpu_memory_gb // self.nb_streams_per_gpu
)
self.resources = Resources(gpus=1.0 / self.nb_streams_per_gpu)
else:
self.resources = Resources(gpus=1)
else:
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ classifiers = [
dependencies = [
"absl-py>=2.0.0,<3.0.0",
"comment_parser",
"cosmos-xenna==0.1.2",
"cosmos-xenna==0.1.8",
"fsspec",
"jieba==0.42.1",
"loguru",
Expand Down
29 changes: 22 additions & 7 deletions tests/stages/video/clipping/test_clip_transcoding_stage.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,6 @@ def __init__(self, index: int, name: str):
self.name = name


# Mock GPU resources class to simulate GPU resources
class MockGpuResources:
def __init__(self, num_nvencs: int = 3, num_nvdecs: int = 3):
self.num_nvencs = num_nvencs
self.num_nvdecs = num_nvdecs


class TestClipTranscodingStage:
"""Test cases for ClipTranscodingStage."""

Expand Down Expand Up @@ -280,6 +273,28 @@ def test_add_hwaccel_options_disabled(self) -> None:
# Should not add any hwaccel options
assert "-hwaccel" not in command

def test_add_hwaccel_options_enabled(self) -> None:
"""Test hardware acceleration options when enabled."""
command = []
stage = ClipTranscodingStage(use_hwaccel=True, encoder="h264_nvenc", nb_streams_per_gpu=1)

stage._add_hwaccel_options(command)

assert "-hwaccel" in command
assert "-hwaccel_output_format" in command
assert stage.resources.gpus == 1.0

def test_add_hwaccel_options_enabled_multiple_streams(self) -> None:
"""Test hardware acceleration options when enabled."""
command = []
stage = ClipTranscodingStage(use_hwaccel=True, encoder="h264_nvenc", nb_streams_per_gpu=4)

stage._add_hwaccel_options(command)

assert "-hwaccel" in command
assert "-hwaccel_output_format" in command
assert stage.resources.gpus == 0.25

def test_add_input_options(self) -> None:
"""Test adding input options to FFmpeg command."""
command = []
Expand Down
Loading
Loading