diff --git a/rl_insight/data/__init__.py b/rl_insight/data/__init__.py index 67e8f83a..f9d0b0ef 100644 --- a/rl_insight/data/__init__.py +++ b/rl_insight/data/__init__.py @@ -14,20 +14,12 @@ """Data module for RL-Insight.""" -from .base import ( - BaseData, - DataValidationError, - ValidationRule, +from .data_checker import ( + DataChecker, + DataEnum ) -from .multi_json import MultiJsonData -from .rules import PathExistsRule -from .enums import DataEnum __all__ = [ - "BaseData", - "MultiJsonData", - "DataValidationError", - "ValidationRule", - "PathExistsRule", + "DataChecker", "DataEnum", ] diff --git a/rl_insight/data/base.py b/rl_insight/data/base.py deleted file mode 100644 index cd9bc241..00000000 --- a/rl_insight/data/base.py +++ /dev/null @@ -1,112 +0,0 @@ -# Copyright (c) 2025 verl-project authors. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -"""Base data definitions for RL-Insight.""" - -from abc import ABC, abstractmethod -from collections.abc import Callable -from typing import List, Optional, Type -from rl_insight.data.enums import DataEnum - -REGISTERED_DATA_ENUM: dict["DataEnum", type["BaseData"]] = {} - - -class DataValidationError(Exception): - """Exception raised when data validation fails.""" - - def __init__(self, message: str, errors: Optional[List[List[str]]] = None): - super().__init__(message) - self.errors = errors or [] - - def __str__(self) -> str: - if self.errors: - return f"{super().__str__()}\n - " + "\n - ".join( - ["\n ".join(err) for err in self.errors] - ) - return super().__str__() - - -class ValidationRule(ABC): - """Validation rule base class""" - - @abstractmethod - def check(self, data: "BaseData") -> bool: - pass - - @property - @abstractmethod - def error_message(self) -> List[str]: - pass - - -class BaseData(ABC): - """Base data class for RL-Insight.""" - - _rules: List[ValidationRule] = [] - """Validation rules for this data class. Should be set by subclasses.""" - _data_type: DataEnum - """Data type for this data class. Should be set by subclasses.""" - - @classmethod - def check(cls, data_type: DataEnum | list[DataEnum], data: "BaseData") -> bool: - """Validate the data""" - - if isinstance(data_type, list): - if cls._data_type not in data_type: - raise DataValidationError( - f"Data type mismatch: expected one of {data_type}, got {cls._data_type}" - ) - else: - if data_type != cls._data_type: - raise DataValidationError( - f"Data type mismatch: expected {cls._data_type}, got {data_type}" - ) - - errors: List[List[str]] = [] - for rule in cls._rules: - if not rule.check(data): - errors.append(rule.error_message) - if errors: - raise DataValidationError("Data validation failed", errors) - return True - - @property - def data_type(self) -> DataEnum: - return self._data_type - - @abstractmethod - def load(self): - """Load the data from source. Should be implemented by subclasses.""" - pass - - -def register_data_cls() -> Callable[[type[BaseData]], type[BaseData]]: - def decorator(cls: type[BaseData]) -> type[BaseData]: - enum = cls._data_type - if enum in REGISTERED_DATA_ENUM: - raise ValueError( - f"Data enum {enum} already registered for {REGISTERED_DATA_ENUM[enum]}" - ) - REGISTERED_DATA_ENUM[enum] = cls - return cls - - return decorator - - -def get_data_cls(data_enum: DataEnum) -> Type[BaseData]: - if data_enum not in REGISTERED_DATA_ENUM: - raise ValueError( - f"Unsupported data enum: {data_enum}. Supported enums are: {list(REGISTERED_DATA_ENUM.keys())}" - ) - return REGISTERED_DATA_ENUM[data_enum] diff --git a/rl_insight/data/data_checker.py b/rl_insight/data/data_checker.py new file mode 100644 index 00000000..11976e35 --- /dev/null +++ b/rl_insight/data/data_checker.py @@ -0,0 +1,61 @@ +# Copyright (c) 2025 verl-project authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Base data definitions for RL-Insight.""" + + +from typing import List +from .rules import ValidationRule, PathExistsRule, DataValidationError +from enum import Enum +import logging +logging.basicConfig( + level=logging.INFO, + format="%(asctime)s - %(name)s - %(levelname)s - %(message)s", + handlers=[logging.StreamHandler()], +) +logger = logging.getLogger(__name__) + + +class DataEnum(Enum): + """Enum for data types in RL-Insight.""" + # input data type of parser + MULTI_JSON = "multi_json" + VERL_LOG = "verl_log" + # output data type of parser, input data type of visualizer + SUMMARY_EVENT = "summary_event" + # other data type + UNKNOWN = "unknown" + + +class DataChecker(): + """Base data class for RL-Insight.""" + rules: dict[DataEnum, List[ValidationRule]] = { + DataEnum.MULTI_JSON: [PathExistsRule()], + DataEnum.SUMMARY_EVENT: [], + } + + def __init__(self, type: DataEnum, data: str|dict): + self.type = type + self.data = data + + def run(self): + """Validate the data""" + errors = [] + rules = self.rules[self.type] + for rule in rules: + if not rule.check(self.data): + errors.append(rule.error_message) + if errors: + raise DataValidationError("Data validation failed", errors) + logger.info(f"Data validation passed for {self.type}") diff --git a/rl_insight/data/enums.py b/rl_insight/data/enums.py deleted file mode 100644 index e2d13d98..00000000 --- a/rl_insight/data/enums.py +++ /dev/null @@ -1,26 +0,0 @@ -# Copyright (c) 2025 verl-project authors. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -from enum import Enum - - -class DataEnum(Enum): - """Enum for data types in RL-Insight.""" - - # input data type of parser - MULTI_JSON = "multi_json" - VERL_LOG = "verl_log" - # output data type of parser, input data type of visualizer - SUMMARY_EVENT = "summary_event" - # other data type - UNKNOWN = "unknown" diff --git a/rl_insight/data/multi_json.py b/rl_insight/data/multi_json.py deleted file mode 100644 index 57f38a9d..00000000 --- a/rl_insight/data/multi_json.py +++ /dev/null @@ -1,33 +0,0 @@ -# Copyright (c) 2025 verl-project authors. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -from dataclasses import dataclass -from pathlib import Path - -from rl_insight.data.base import BaseData, register_data_cls -from rl_insight.data.enums import DataEnum -from rl_insight.data.rules import PathExistsRule - - -@register_data_cls() -@dataclass -class MultiJsonData(BaseData): - path: Path - - _rules = [PathExistsRule()] - _data_type = DataEnum.MULTI_JSON - - def __post_init__(self): - if isinstance(self.path, str): - self.path = Path(self.path) diff --git a/rl_insight/data/rules.py b/rl_insight/data/rules.py index 4b423e20..63897613 100644 --- a/rl_insight/data/rules.py +++ b/rl_insight/data/rules.py @@ -13,21 +13,56 @@ # limitations under the License. from typing import List +from abc import ABC, abstractmethod +from pathlib import Path +from typing import Optional +import pandas as pd -from rl_insight.data.base import ValidationRule -class PathExistsRule(ValidationRule): - _error_message: List[str] = [] +class DataValidationError(Exception): + """Exception raised when data validation fails.""" + + def __init__(self, message: str, errors: Optional[List[List[str]]] = None): + super().__init__(message) + self.errors = errors or [] + + def __str__(self) -> str: + if self.errors: + return f"{super().__str__()}\n - " + "\n - ".join( + ["\n ".join(err) for err in self.errors] + ) + return super().__str__() + +class ValidationRule(ABC): + """Validation rule base class""" + + @abstractmethod def check(self, data) -> bool: - if not hasattr(data, "path"): - self._error_message = ["Data object does not have 'path' attribute"] + pass + + @property + @abstractmethod + def error_message(self) -> List[str]: + pass + + +class PathExistsRule(ValidationRule): + _error_message: str + def check(self, data: str|dict|pd.DataFrame) -> bool: + if not isinstance(data, str): + self._error_message = "Data object is not a path" return False - if not data.path.exists(): - self._error_message = [f"Source path does not exist: {data.path}"] + try: + path = Path(data) + if not path.is_dir(): + self._error_message = f"Source path does not exist: {data}" + return False + return True + except Exception as e: + self._error_message = f"Source path does not exist: {data}" return False - return True @property def error_message(self) -> List[str]: diff --git a/rl_insight/data/summary_event.py b/rl_insight/data/summary_event.py deleted file mode 100644 index 8be1d6cb..00000000 --- a/rl_insight/data/summary_event.py +++ /dev/null @@ -1,13 +0,0 @@ -# Copyright (c) 2025 verl-project authors. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. diff --git a/rl_insight/data/verl_log.py b/rl_insight/data/verl_log.py deleted file mode 100644 index 8be1d6cb..00000000 --- a/rl_insight/data/verl_log.py +++ /dev/null @@ -1,13 +0,0 @@ -# Copyright (c) 2025 verl-project authors. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. diff --git a/rl_insight/main.py b/rl_insight/main.py index 5f1dc036..1e640bd2 100644 --- a/rl_insight/main.py +++ b/rl_insight/main.py @@ -29,7 +29,7 @@ def run_pipeline(config, pipeline_class=None): def main(): arg_parser = argparse.ArgumentParser(description="Cluster scheduling visualization") arg_parser.add_argument( - "--input-path", default="test", help="Raw path of profiling data" + "--input-path", default=r"C:\Users\Tardis\Documents\profile_data\discrete_analyse_false_json_only", help="Raw path of profiling data" ) arg_parser.add_argument( "--input-type", diff --git a/rl_insight/parser/parser.py b/rl_insight/parser/parser.py index 2bb5f222..be21dc69 100644 --- a/rl_insight/parser/parser.py +++ b/rl_insight/parser/parser.py @@ -20,8 +20,7 @@ import pandas as pd -from rl_insight.data.base import BaseData -from rl_insight.data.enums import DataEnum +from rl_insight.data import DataEnum from rl_insight.utils.schema import Constant, DataMap, EventRow logging.basicConfig( @@ -34,8 +33,8 @@ class BaseClusterParser(ABC): def __init__(self, params) -> None: + self.input_type = DataEnum.MULTI_JSON self.events_summary: Optional[pd.DataFrame] = None - self.input_path = params.get(Constant.INPUT_PATH, "") rank_list = params.get(Constant.RANK_LIST, "all") self._rank_list = ( rank_list @@ -43,9 +42,9 @@ def __init__(self, params) -> None: else [int(rank) for rank in rank_list.split(",") if rank.isdigit()] ) - def run(self) -> BaseData: + def run(self, input_data: str) -> pd.DataFrame: """Run parsing and return the parsed DataFrame.""" - _data_maps = self.allocate_prof_data(self.input_path) + _data_maps = self.allocate_prof_data(input_data) mapper_res = self.mapper_func(_data_maps) self.reducer_func(mapper_res) return self.get_data() @@ -132,14 +131,6 @@ def clean_data(self) -> None: def get_data(self) -> pd.DataFrame: return self.events_summary - def get_input_type(self) -> List[DataEnum]: - """Return a list of acceptable input data types for this parser.""" - return [DataEnum.MULTI_JSON] - - def get_output_type(self) -> DataEnum: - """Return the output data type produced by this parser.""" - return DataEnum.SUMMARY_EVENT - @abstractmethod def allocate_prof_data(self, input_path: str) -> list[DataMap]: """ diff --git a/rl_insight/pipeline/offline_insight_pipeline.py b/rl_insight/pipeline/offline_insight_pipeline.py index 270ccde9..ba66d9e6 100644 --- a/rl_insight/pipeline/offline_insight_pipeline.py +++ b/rl_insight/pipeline/offline_insight_pipeline.py @@ -12,8 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -from rl_insight.data.base import get_data_cls -from rl_insight.data.enums import DataEnum +from rl_insight.data import DataChecker, DataEnum from rl_insight.parser import get_cluster_parser_cls from rl_insight.utils.schema import Constant from rl_insight.visualizer.visualizer import RLTimelineVisualizer @@ -21,54 +20,42 @@ class OfflineInsightPipeline: def __init__(self, config): - self.input_path = config.input_path - self.profiler_type = config.profiler_type - self.output_path = config.output_path - self.vis_type = config.vis_type - self.rank_list = config.rank_list - self.input_type: DataEnum = DataEnum(config.input_type) - self.input_data_cls = get_data_cls(self.input_type) + self.config = config + + # init data + self.input_data_type = DataEnum(self.config.input_type) # parser related - self.parser_config = self._prepare_parser_config() - self.parser_cls = get_cluster_parser_cls(self.profiler_type) - self.parser = self.parser_cls(self.parser_config) - self.parser_input_type = self.parser.get_input_type() - self.parser_output_type = self.parser.get_output_type() + parser_config = self._prepare_parser_config() + parser_cls = get_cluster_parser_cls(self.config.profiler_type) + self.parser = parser_cls(parser_config) # visualizer related - self.visualizer_config = self._prepare_visualizer_config() - self.visualizer = RLTimelineVisualizer(self.visualizer_config) - self.visualizer_input_type = self.visualizer.get_input_type() + visualizer_config = self._prepare_visualizer_config() + self.visualizer = RLTimelineVisualizer(visualizer_config) def _prepare_parser_config(self): return { - Constant.INPUT_PATH: self.input_path, - Constant.RANK_LIST: self.rank_list, + Constant.RANK_LIST: self.config.rank_list, } def _prepare_visualizer_config(self): return { - "output_path": self.output_path, - "vis_type": self.vis_type, + "output_path": self.config.output_path, + "vis_type": self.config.vis_type, } - def _input_data_check(self): - """Check if the input data is valid for the parser.""" - if self.input_type not in self.parser_input_type: - raise ValueError( - f"Input data type {self.input_type} does not match parser input type {self.parser_input_type}" - ) - - def _inter_res_check(self): - """Check if the intermediate results from parser are valid for visualizer.""" - if self.parser_output_type not in self.visualizer_input_type: + def run(self): + if self.input_data_type != self.parser.input_type: raise ValueError( - f"Parser output type {self.parser_output_type} does not match visualizer input type {self.visualizer_input_type}" + f"Input data type {self.input_data_type} does not match parser input type {self.parser.input_type}" ) - - def run(self): - self._input_data_check() - self._inter_res_check() - data = self.parser.run() - self.visualizer.run(data) + # validate input data + DataChecker(self.input_data_type, self.config.input_path).run() + + output_data = self.parser.run(self.config.input_path) + + # validate output data + DataChecker(self.visualizer.input_type, output_data).run() + + self.visualizer.run(output_data) diff --git a/rl_insight/visualizer/visualizer.py b/rl_insight/visualizer/visualizer.py index e05f61cd..4b09fa27 100644 --- a/rl_insight/visualizer/visualizer.py +++ b/rl_insight/visualizer/visualizer.py @@ -20,7 +20,7 @@ import numpy as np import pandas as pd import plotly.graph_objects as go -from rl_insight.data.enums import DataEnum +from rl_insight.data import DataEnum from rl_insight.utils.schema import FigureConfig logging.basicConfig( @@ -80,6 +80,7 @@ def __init__(self, config: dict): super().__init__(config) self.output_path = config.get("output_path", None) self.vis_type = config.get("vis_type", None) + self.input_type = DataEnum.SUMMARY_EVENT self.visualizer_fn = None def get_input_type(self) -> List[DataEnum]: diff --git a/tests/cluster_analysis/test_cluster_analysis.py b/tests/cluster_analysis/test_cluster_analysis.py index 065223d9..34613176 100644 --- a/tests/cluster_analysis/test_cluster_analysis.py +++ b/tests/cluster_analysis/test_cluster_analysis.py @@ -29,7 +29,7 @@ import pandas as pd import pytest -from rl_insight.data.enums import DataEnum +from rl_insight.data import DataEnum from rl_insight.main import main from rl_insight.parser import MstxClusterParser from rl_insight.parser import ( diff --git a/tests/cluster_analysis/test_data.py b/tests/cluster_analysis/test_data.py deleted file mode 100644 index f97a3ac7..00000000 --- a/tests/cluster_analysis/test_data.py +++ /dev/null @@ -1,85 +0,0 @@ -# Copyright (c) 2025 verl-project authors. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -"""Integration tests for data module.""" - -from dataclasses import dataclass -from pathlib import Path - -import pytest - -from rl_insight.data.base import BaseData, register_data_cls, get_data_cls -from rl_insight.data.enums import DataEnum -from rl_insight.data.rules import PathExistsRule - - -@register_data_cls() -@dataclass -class SampleData(BaseData): - path: Path - - _data_type = DataEnum.UNKNOWN - _rules = [PathExistsRule()] - _content: str = "" - - def __post_init__(self): - if isinstance(self.path, str): - self.path = Path(self.path) - - def load(self): - with open(self.path, "r") as f: - self._content = f.read() - - @property - def content(self): - if not self._content: - self.load() - return self._content - - -@pytest.fixture -def sample_data(tmp_path): - # Create a temporary file for testing - test_file = tmp_path / "test_file.txt" - test_file.write_text("This is a test file.") - return SampleData(path=test_file) - - -class TestSampleData: - def test_registration(self): - data_cls = get_data_cls(DataEnum.UNKNOWN) - assert data_cls is SampleData - - def test_path_not_exists(self, tmp_path=Path("/tmp")): - data = SampleData(path=tmp_path / "non_existent_file.txt") - try: - SampleData.check(data_type=DataEnum.UNKNOWN, data=data) - except Exception as e: - assert isinstance(e, Exception) - assert "Data validation failed" in str(e) - assert "path does not exist" in str(e) - - def test_enum_mismatch(self, sample_data): - try: - SampleData.check(data_type=DataEnum.MULTI_JSON, data=sample_data) - except Exception as e: - assert isinstance(e, Exception) - assert "Data type mismatch" in str(e) - - def test_valid_data(self, sample_data): - try: - SampleData.check(data_type=DataEnum.UNKNOWN, data=sample_data) - assert sample_data.content == "This is a test file." - except Exception as e: - pytest.fail(f"Unexpected exception raised: {e}") diff --git a/tests/data/test_data_checker.py b/tests/data/test_data_checker.py new file mode 100644 index 00000000..61a9a042 --- /dev/null +++ b/tests/data/test_data_checker.py @@ -0,0 +1,21 @@ +import pytest + +from rl_insight.data.data_checker import DataChecker, DataEnum +from rl_insight.data.rules import DataValidationError + + +def test_data_checker_multi_json_path_exists(tmp_path): + checker = DataChecker(type=DataEnum.MULTI_JSON, data=str(tmp_path)) + checker.run() + + +def test_data_checker_multi_json_path_missing(): + checker = DataChecker(type=DataEnum.MULTI_JSON, data="C:/definitely/not/exist/path") + with pytest.raises(DataValidationError) as exc_info: + checker.run() + assert "Data validation failed" in str(exc_info.value) + + +def test_data_checker_summary_event_has_no_rule_with_dict_data(): + checker = DataChecker(type=DataEnum.SUMMARY_EVENT, data={"k": "v"}) + checker.run() diff --git a/tests/data/test_rules.py b/tests/data/test_rules.py new file mode 100644 index 00000000..ade0630e --- /dev/null +++ b/tests/data/test_rules.py @@ -0,0 +1,27 @@ +import pytest + +from rl_insight.data.rules import DataValidationError, PathExistsRule + + +def test_path_exists_rule_accepts_existing_directory(tmp_path): + rule = PathExistsRule() + assert rule.check(str(tmp_path)) is True + + +def test_path_exists_rule_rejects_non_string_input(): + rule = PathExistsRule() + assert rule.check({"path": "x"}) is False + assert "not a path" in rule.error_message + + +def test_path_exists_rule_rejects_missing_directory(): + rule = PathExistsRule() + assert rule.check("C:/definitely/not/exist/path") is False + + +def test_data_validation_error_string_includes_error_details(): + err = DataValidationError("Data validation failed", [["line1", "line2"]]) + text = str(err) + assert "Data validation failed" in text + assert "line1" in text + assert "line2" in text