Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions projects/rocprofiler-compute/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ Full documentation for ROCm Compute Profiler is available at [https://rocm.docs.

### Removed

* Removed the multi-node analysis options ``--nodes``, ``--list-nodes`` (analyze mode) and the experimental ``--spatial-multiplexing`` option (profile and analyze modes). These features did not work as expected and will be redesigned in a future release.

* ``--path`` and ``--subpath`` options have been removed from profile mode. Use ``--output-directory`` instead.

* Removed redundant `if (X != 0) else None` divide-by-zero guards from metric equations across all analysis YAML configurations. Division by zero is already handled by the metric evaluation engine, which returns `"N/A"` for `inf` and `NaN` results.
Expand Down
27 changes: 15 additions & 12 deletions projects/rocprofiler-compute/PYTHON_CODING_STYLE.md
Original file line number Diff line number Diff line change
Expand Up @@ -378,17 +378,18 @@ Each function should do **ONE** thing well. If you use "and" to describe what it

```python
def create_df_pmc(
raw_data_root_dir: str,
nodes: Optional[list[str]],
spatial_multiplexing: bool,
raw_data_dir: str,
kernel_verbose: int,
verbose: int,
config_dict: dict[str, Any],
) -> pd.DataFrame:
"""Load all raw pmc counters and join into one dataframe."""
# Single responsibility: create and return a DataFrame.
# Delegates the details to a focused helper.
return _create_single_df_pmc(...)
# Single responsibility: load counters into a DataFrame and return it.
df = pd.read_csv(Path(raw_data_dir) / "pmc_perf.csv")
if config_dict.get("format_rocprof_output") == "rocpd":
df = utils_analysis.process_rocpd_csv(df)
kernel_name_shortener(df, kernel_verbose)
return df
```

**Bad:** Multiple responsibilities
Expand Down Expand Up @@ -427,9 +428,11 @@ def pre_processing(self) -> None:
# Each operation delegated to a focused helper
workload.raw_pmc = file_io.create_df_pmc(...)

if args.spatial_multiplexing:
workload.raw_pmc = self.spatial_multiplex_merge_counters(
workload.raw_pmc
if self._profiling_config.get("iteration_multiplexing") is not None:
workload.raw_pmc = self.iteration_multiplex_impute_counters(
workload.raw_pmc,
policy=self._profiling_config["iteration_multiplexing"],
workload_dir=Path(path_info[0]),
)

file_io.create_df_kernel_top_stats(...)
Expand All @@ -452,9 +455,9 @@ def pre_processing(self) -> None:
for csv_file in Path(path_info[0]).rglob("*.csv"):
# ... lots of processing

# 30 lines of inline merge logic
if args.spatial_multiplexing:
# ... complex merging
# 30 lines of inline imputation logic
if self._profiling_config.get("iteration_multiplexing"):
# ... complex counter imputation

# 40 lines of inline stats creation
# ... more processing
Expand Down
46 changes: 0 additions & 46 deletions projects/rocprofiler-compute/src/argparser.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ def add_general_group(
"Enable experimental feature(s):\n"
" GUI (--gui)\n"
" TUI (--tui)\n"
" Spatial multiplexing (--spatial-multiplexing)\n"
" Torch trace (--torch-trace, --list-torch-operators, --torch-operator)\n"
" PC Sampling (--pc-sampling, --pc-sampling-method, "
"--pc-sampling-interval)\n"
Expand Down Expand Up @@ -541,21 +540,6 @@ def omniarg_parser(
# Experimental Features
## ----------------------------

profile_group.add_argument(
"--spatial-multiplexing",
dest="spatial_multiplexing",
required=False,
default=None,
base_action="store",
action=ExperimentalAction,
experimental_enabled=experimental_enabled,
feature_label="Spatial multiplexing",
type=int,
nargs="*",
metavar="",
help="\t\t\tProvide Node ID and GPU number per node.",
)

profile_group.add_argument(
"--membw-analysis",
dest="membw_analysis",
Expand Down Expand Up @@ -972,40 +956,10 @@ def omniarg_parser(
help="\t\tSpecify the specs to correct. e.g. "
'--specs-correction="specname1:specvalue1,specname2:specvalue2"',
)
analyze_advanced_group.add_argument(
"--list-nodes",
action="store_true",
help="\t\tMulti-node option: list all node names.",
)
analyze_advanced_group.add_argument(
"--nodes",
metavar="",
type=str,
dest="nodes",
nargs="*",
help=(
"\t\tMulti-node option: filter with node names. "
"Enable it without node names means ALL."
),
)

## ----------------------------
# Experimental Features
## ----------------------------
analyze_group.add_argument(
"--spatial-multiplexing",
dest="spatial_multiplexing",
required=False,
default=False,
base_action="store_const",
action=ExperimentalAction,
experimental_enabled=experimental_enabled,
feature_label="Spatial multiplexing",
nargs=0,
const=True,
help="\t\tMode of spatial multiplexing.",
)

analyze_group.add_argument(
"--membw-analysis",
dest="membw_analysis",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
from utils.utils_analysis import (
impute_counters_iteration_multiplex,
is_workload_empty,
merge_counters_spatial_multiplex,
)
from utils.utils_common import (
PC_SAMPLING_BLOCK_IDS,
Expand Down Expand Up @@ -116,7 +115,6 @@ def build_pc_sampling_only_workload(
dir_path: str,
args: argparse.Namespace,
tool_data: Optional[dict[str, Any]],
filter_nodes: Optional[list[str]] = None,
) -> None:
"""Build dispatch scaffolding and tables for a run without counters."""
workload.raw_pmc = file_io.process_pc_sampling_kernel_trace(tool_data)
Expand All @@ -128,9 +126,6 @@ def build_pc_sampling_only_workload(
raw_data_dir=str(dir_path),
filter_gpu_ids=workload.filter_gpu_ids,
filter_dispatch_ids=workload.filter_dispatch_ids,
filter_nodes=(
filter_nodes if filter_nodes is not None else workload.filter_nodes
),
time_unit=args.time_unit,
kernel_verbose=args.kernel_verbose,
)
Expand All @@ -147,10 +142,6 @@ def set_soc(self, omni_socs: dict[str, OmniSoC_Base]) -> None:
def get_socs(self) -> Optional[dict[str, OmniSoC_Base]]:
return self.__socs

@demarcate
def spatial_multiplex_merge_counters(self, df: pd.DataFrame) -> pd.DataFrame:
return merge_counters_spatial_multiplex(df)

@demarcate
def iteration_multiplex_impute_counters(
self, df: pd.DataFrame, policy: str, workload_dir: Path
Expand Down Expand Up @@ -230,16 +221,9 @@ def initalize_runs(
) -> OrderedDict[str, schema.Workload]:
args = self.get_args()

def get_sysinfo_path(data_path: str) -> Optional[str]:
return (
data_path
if args.nodes is None and not args.spatial_multiplexing
else file_io.find_1st_sub_dir(data_path)
)

# load required configs
for path_info in args.path:
sysinfo_path = get_sysinfo_path(path_info[0])
sysinfo_path = path_info[0]
if sysinfo_path:
sys_info = pd.read_csv(f"{sysinfo_path}/sysinfo.csv")
arch = sys_info.iloc[0]["gpu_arch"]
Comment on lines +226 to 229
Expand All @@ -255,12 +239,8 @@ def get_sysinfo_path(data_path: str) -> Optional[str]:
self.load_options(normalization_filter)

for path_info in args.path:
# FIXME:
# For regular single node case, load sysinfo.csv directly
# For multi-node, either the default "all", or specified some,
# pick up the one in the 1st sub_dir. We could fix it properly later.
w = schema.Workload()
sysinfo_path = get_sysinfo_path(path_info[0])
sysinfo_path = path_info[0]
if sysinfo_path:
w.sys_info = pd.read_csv(f"{sysinfo_path}/sysinfo.csv")
if not getattr(args, "no_roof", False):
Comment on lines 242 to 246
Expand Down Expand Up @@ -353,66 +333,11 @@ def sanitize(self) -> None:

for dir_info in args.path:
if not any([
args.nodes,
args.list_nodes,
args.spatial_multiplexing,
profiling_config.get("iteration_multiplexing"),
self.pc_sampling_only(),
]):
is_workload_empty(dir_info[0])

# FIXME:
# The proper location of this func should be in pre_processing().
# However, because of reading soc depends on sys spec, and sys
# spec depends on sys_info. And we read sys_info too early so we
# . can not do it now. There should be a way to make it simpler.
if args.list_nodes:
# NB:
# There are 2 ways to do it: one is doing like the below, checking
# sub dirs only as we assume the profiling stage generate sub dirs
# with node name. The 2nd way would be checkign host name in each
# sub dir and very those.
nodes = [
subdir.name
for subdir in Path(args.path[0][0]).iterdir()
if subdir.is_dir()
]
print("Node list:", " ".join(nodes))
sys.exit(0)

# Validate --nodes option against workload structure
if args.nodes is not None:
for dir_info in args.path:
workload_path = dir_info[0]
valid_nodes = file_io.get_valid_nodes(workload_path)

if not valid_nodes:
# Single-node workload: sysinfo.csv is in root, not in
# subdirectories
console_error(
"analysis",
f"The workload at '{workload_path}' is single-node "
"(sysinfo.csv is in the root directory).\n"
"The --nodes option is only supported for multi-node "
"workloads where each node subdirectory contains its "
"own sysinfo.csv.\n"
"Remove the --nodes option to analyze this "
"single-node workload.",
)

# If specific nodes are provided (not empty list), validate them
if args.nodes:
invalid_nodes = [n for n in args.nodes if n not in valid_nodes]
if invalid_nodes:
console_error(
"analysis",
f"Invalid node(s): {', '.join(invalid_nodes)}\n"
f"Valid nodes for '{workload_path}': "
f"{', '.join(valid_nodes)}\n"
"Each valid node must be a subdirectory "
"containing sysinfo.csv.",
)

# Ensure analysis output does not overwrite existing files
if args.output_name:
if not re.match(r"^[A-Za-z0-9_-]+$", args.output_name):
Expand Down Expand Up @@ -681,47 +606,26 @@ def join_prof(
return None

def join_workload_csvs(self, workload_dir: Path) -> None:
"""Join CSV files for a workload directory.

Handles multi-node and spatial multiplexing.

This method checks if the workload uses multi-node or spatial multiplexing,
and joins CSV files accordingly:
- Multi-node/spatial: Joins CSV files in each subdirectory (0/, 1/, 2/, etc.)
- Regular single-node: Joins CSV files in the workload directory directly
"""Join results_*.csv source files into pmc_perf.csv if needed.

Args:
workload_dir: Path to the workload directory
"""
args = self.get_args()

# Helper to process and join CSV files in a single directory
def process_and_join_directory(directory: Path) -> None:
pmc_perf = directory / "pmc_perf.csv"
results_files = list(directory.glob("results_*.csv"))

if pmc_perf.exists():
console_debug(f"Using existing {pmc_perf}")
elif results_files:
console_log(f"Joining results_*.csv for {directory}...")
self.join_prof(directory, out=str(pmc_perf))
console_log(f"Created {pmc_perf}")
else:
console_error(
f"No profiling data found in {directory}.\n"
f"Expected: pmc_perf.csv or results_*.csv\n"
f"Please run 'rocprof-compute profile' first."
)

# Handle multi-node and spatial multiplexing cases
if args.nodes is not None or args.spatial_multiplexing:
# Multi-node or spatial case: CSV files are in subdirectories
for subdir in workload_dir.iterdir():
if subdir.is_dir():
process_and_join_directory(subdir)
pmc_perf = workload_dir / "pmc_perf.csv"
results_files = list(workload_dir.glob("results_*.csv"))

if pmc_perf.exists():
console_debug(f"Using existing {pmc_perf}")
elif results_files:
console_log(f"Joining results_*.csv for {workload_dir}...")
self.join_prof(workload_dir, out=str(pmc_perf))
console_log(f"Created {pmc_perf}")
else:
# Regular single-node case: CSV files are in workload_dir directly
process_and_join_directory(workload_dir)
console_error(
f"No profiling data found in {workload_dir}.\n"
f"Expected: pmc_perf.csv or results_*.csv\n"
f"Please run 'rocprof-compute profile' first."
)

# ----------------------------------------------------
# Required methods to be implemented by child classes
Expand Down Expand Up @@ -750,7 +654,6 @@ def pre_processing(self) -> None:
(args.gpu_kernel, "filter_kernel_ids"),
(args.gpu_id, "filter_gpu_ids"),
(args.gpu_dispatch_id, "filter_dispatch_ids"),
(args.nodes, "filter_nodes"),
]

for filter_list, attr_name in filter_configs:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,18 +80,11 @@ def pre_processing(self) -> None:
# create 'mega dataframe'
workload.raw_pmc = file_io.create_df_pmc(
path_info[0],
args.nodes,
args.spatial_multiplexing,
args.kernel_verbose,
args.verbose,
self._profiling_config,
)

if args.spatial_multiplexing:
workload.raw_pmc = self.spatial_multiplex_merge_counters(
workload.raw_pmc
)

if self._profiling_config.get("iteration_multiplexing") is not None:
workload.raw_pmc = self.iteration_multiplex_impute_counters(
workload.raw_pmc,
Expand All @@ -104,7 +97,6 @@ def pre_processing(self) -> None:
raw_data_dir=path_info[0],
filter_gpu_ids=workload.filter_gpu_ids,
filter_dispatch_ids=workload.filter_dispatch_ids,
filter_nodes=workload.filter_nodes,
time_unit=args.time_unit,
kernel_verbose=args.kernel_verbose,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,6 @@ def run_analysis_metrics(

def calc_pmc_df_data(self) -> dict[str, pd.DataFrame]:
pmc_df_per_workload: dict[str, pd.DataFrame] = {}
args = self.get_args()

for workload_path in self._runs.keys():
if not (Path(workload_path) / "pmc_perf.csv").exists():
Expand All @@ -327,9 +326,6 @@ def calc_pmc_df_data(self) -> dict[str, pd.DataFrame]:
pd.read_csv(Path(workload_path) / "pmc_perf.csv")
)

if args.spatial_multiplexing:
pmc_df = self.spatial_multiplex_merge_counters(pmc_df)

if self._profiling_config.get("iteration_multiplexing") is not None:
pmc_df = self.iteration_multiplex_impute_counters(
pmc_df,
Expand Down
Loading
Loading