From ed1bed9c67e249a842a55ed9ea0ab5b5bfb61a10 Mon Sep 17 00:00:00 2001 From: surya periaswamy Date: Tue, 2 Jun 2026 21:59:34 -0500 Subject: [PATCH 01/17] AIMVT-196: fix RCCL regression group-by bug + add paired A/B detector Fix the group-by/key bug in the RCCL regression pipeline where results were bucketed by message size alone, silently collapsing the (data type, inPlace) dimensions and hiding regressions: - add group_rccl_results() canonical grouping by (type, inPlace) + size sort - convert_to_graph_dict no longer overwrites in-place/out-of-place/dtype rows - check_bw_dip / check_lat_dip / check_bus_bw group + sort before comparing, eliminating dtype-boundary false positives and cross-dimension mixups Add a paired A/B regression detector (regression_lib.py) that compares a candidate build against a reference build run back-to-back on the same nodes, so common-mode noise cancels. Triple-gated for CI stability: size-tiered relative threshold + non-parametric separation (p75(B) < p25(A)) + adjacency confirmation, plus on-hardware threshold derivation from a control run. Validated on 4-node MI350X: 0 false positives on A=A control, real regressions caught on ref-vs-candidate. Add rccl_ab_regression.py orchestration, optional per-side librccl path + -d data-type support in rccl_regression, plm_rsh_args for multi-node ssh launch, and unit tests (47 passing). Co-authored-by: Cursor --- .../rccl/rccl_ab_config.json.sample | 68 +++ cvs/lib/rccl_lib.py | 288 ++++++----- cvs/lib/regression_lib.py | 483 ++++++++++++++++++ cvs/lib/unittests/test_rccl_lib.py | 112 ++++ cvs/lib/unittests/test_regression_lib.py | 274 ++++++++++ cvs/tests/rccl/rccl_ab_regression.py | 315 ++++++++++++ 6 files changed, 1416 insertions(+), 124 deletions(-) create mode 100644 cvs/input/config_file/rccl/rccl_ab_config.json.sample create mode 100644 cvs/lib/regression_lib.py create mode 100644 cvs/lib/unittests/test_regression_lib.py create mode 100644 cvs/tests/rccl/rccl_ab_regression.py diff --git a/cvs/input/config_file/rccl/rccl_ab_config.json.sample b/cvs/input/config_file/rccl/rccl_ab_config.json.sample new file mode 100644 index 000000000..b1bd9b472 --- /dev/null +++ b/cvs/input/config_file/rccl/rccl_ab_config.json.sample @@ -0,0 +1,68 @@ +{ + "rccl": { + "mpi_params": { + "no_of_nodes": "1", + "no_of_local_ranks": "8", + "mpi_pml": "auto", + "mpi_dir": "/apps/sp/ompi-install", + "mpi_oob_port": "eth0" + }, + + "env_source_script": "None", + + "rccl_test_params": { + "rccl_tests_dir": "/apps/sp/rccl-tests/build", + "start_msg_size": "1024", + "end_msg_size": "4G", + "step_function": "2", + "threads_per_gpu": "1", + "warmup_iterations": "10", + "no_of_iterations": "20", + "no_of_cycles": "1", + "check_iteration_count": "1", + "rccl_timeout": "1800", + "output_algo_proto_channels": false + }, + + "cvs_params": { + "cluster_snapshot_debug": "False", + "nic_model": "thor", + "cvs_exec_timeout": "3600", + "rccl_result_file": "/tmp/rccl_ab_result.json" + }, + + "_comment_regression": "NCCL env-combos to sweep. Keep small at first; each combo runs A and B x repeats.", + "regression": { + "NCCL_ALGO": [ "Ring" ], + "NCCL_PROTO": [ "Simple" ] + }, + + "rccl_collective": [ "all_reduce_perf" ], + + "_comment_ab": "Paired A/B regression settings. control_mode=true runs the reference build as BOTH sides to measure on-hardware noise and derive thresholds (writes ab_derived_thresholds.json). Set control_mode=false with distinct builds for real detection.", + "ab_regression": { + "repeats": 7, + "control_mode": true, + "safety_factor": 2.0, + "adjacency_min_run": 2, + "min_repeats": 2, + "min_bandwidth_floor": 0.5, + "metric": "busBw", + "higher_is_better": true, + "output_dir": "/tmp/cvs_ab", + "reference": { + "label": "ref", + "rccl_tests_dir": "/apps/sp/rccl-tests/build", + "ld_library_path": "/apps/sp/rccl/build/release" + }, + "candidate": { + "label": "cand", + "rccl_tests_dir": "/apps/sp/rccl-tests/build", + "ld_library_path": "/apps/sp/rccl/build/release" + }, + "_comment_thresholds": "Optional fixed thresholds. In control_mode these are overwritten by derived values.", + "thresholds": { "small": 0.20, "mid": 0.10, "large": 0.05 }, + "tier_boundaries": { "small_max_bytes": 1048576, "mid_max_bytes": 67108864 } + } + } +} diff --git a/cvs/lib/rccl_lib.py b/cvs/lib/rccl_lib.py index 1240f5698..b314e105e 100644 --- a/cvs/lib/rccl_lib.py +++ b/cvs/lib/rccl_lib.py @@ -341,38 +341,34 @@ def check_bus_bw(test_name, output, exp_res_dict): tolerance = 0.95 # 5% tolerance - # New hierarchical structure: {msg_size: {'bus_bw': bw_value}} - msg_size_list = list(exp_res_dict.keys()) + # Reference structure: {msg_size: {'bus_bw': bw_value}}. The reference carries + # no data-type dimension, so we look it up purely by message size and only + # compare the rows for the inPlace orientation that is meaningful for this + # collective. Grouping by (type, inPlace) keeps each data type's rows distinct + # so the comparison is reported per data type instead of being silently mixed. + ref_by_size = {str(size): float(metrics['bus_bw']) for size, metrics in exp_res_dict.items()} log.info("%s", test_name) - # act_res_dict = json.loads(output.replace( '\n', '').replace( '\r', '')) - act_res_dict = output - if re.search('alltoall|all_to_all', test_name, re.I): - for act_dict in act_res_dict: - if act_dict['inPlace'] == 0: - for msg_size in msg_size_list: - if str(msg_size) == str(act_dict['size']): - expected_bw = float(exp_res_dict[msg_size]['bus_bw']) - actual_bw = float(act_dict['busBw']) - threshold = expected_bw * tolerance - log.info(f"Comparing: actual={actual_bw}, expected={expected_bw}, threshold={threshold:.2f}") - if actual_bw < threshold: - fail_test( - f"The actual out-of-place bus BW {actual_bw} for msg size {act_dict['size']} is lower than expected bus BW {expected_bw} (threshold with 5% tolerance: {threshold:.2f})" - ) - else: - for act_dict in act_res_dict: - if act_dict['inPlace'] == 1: - for msg_size in msg_size_list: - if str(msg_size) == str(act_dict['size']): - expected_bw = float(exp_res_dict[msg_size]['bus_bw']) - actual_bw = float(act_dict['busBw']) - threshold = expected_bw * tolerance - log.info(f"Comparing: actual={actual_bw}, expected={expected_bw}, threshold={threshold:.2f}") - if actual_bw < threshold: - fail_test( - f"The actual in-place bus BW {actual_bw} for msg size {act_dict['size']} is lower than expected bus BW {expected_bw} (threshold with 5% tolerance: {threshold:.2f})" - ) + place_word = 'out-of-place' if re.search('alltoall|all_to_all', test_name, re.I) else 'in-place' + target_inplace = 0 if place_word == 'out-of-place' else 1 + + for (dtype, in_place), rows in group_rccl_results(output).items(): + if in_place != target_inplace: + continue + for act_dict in rows: + expected_bw = ref_by_size.get(str(act_dict['size'])) + if expected_bw is None: + continue + actual_bw = float(act_dict['busBw']) + threshold = expected_bw * tolerance + log.info( + f"Comparing (type={dtype}): actual={actual_bw}, expected={expected_bw}, threshold={threshold:.2f}" + ) + if actual_bw < threshold: + fail_test( + f"The actual {place_word} bus BW {actual_bw} for msg size {act_dict['size']} (type={dtype}) " + f"is lower than expected bus BW {expected_bw} (threshold with 5% tolerance: {threshold:.2f})" + ) def check_bw_dip(test_name, output, exp_res_dict=None): @@ -380,13 +376,13 @@ def check_bw_dip(test_name, output, exp_res_dict=None): Check for bandwidth dips as message size increases. Only fails if bandwidth drops by more than 5%. Only validates message sizes specified in the reference. If no reference provided, skips validation. - """ - # act_res_dict = json.loads(output.replace( '\n', '').replace( '\r', '')) - act_res_dict = output - tolerance = 0.95 # 5% tolerance - # Get reference message sizes if provided - # If no reference data, skip validation entirely + Rows are grouped by (data type, inPlace) and sorted ascending by message size + before the consecutive comparison. Without this grouping a result list that + contains more than one data type would compare the largest size of one data + type against the smallest size of the next, manufacturing a spurious "dip" + at every data-type boundary (a false positive). + """ if not exp_res_dict: log.warning(f"No reference data provided for BW dip check, skipping validation for {test_name}") return @@ -394,40 +390,30 @@ def check_bw_dip(test_name, output, exp_res_dict=None): ref_msg_sizes = set(str(size) for size in exp_res_dict.keys()) log.info(f"Validating BW dip only for reference message sizes: {ref_msg_sizes}") - if re.search('alltoall|all_to_all', test_name, re.I): - last_bw = 0.0 - last_msg_size = act_res_dict[0]['size'] - for act_dict in act_res_dict: - if act_dict['inPlace'] == 0: - # Skip validation if this message size is not in reference - if str(act_dict['size']) not in ref_msg_sizes: - continue - - current_bw = float(act_dict['busBw']) - threshold = float(last_bw) * tolerance - if last_bw > 0 and current_bw < threshold: - fail_test( - f"The BusBW for msg size {act_dict['size']} = {current_bw} is less than the earlier msg size {last_msg_size} = BW {last_bw} (threshold with 5% tolerance: {threshold:.2f})" - ) - last_bw = act_dict['busBw'] - last_msg_size = act_dict['size'] - else: + # alltoall reports the meaningful number out-of-place (inPlace == 0); every + # other collective is validated in-place (inPlace == 1). + target_inplace = 0 if re.search('alltoall|all_to_all', test_name, re.I) else 1 + tolerance = 0.95 # 5% tolerance + + for (dtype, in_place), rows in group_rccl_results(output).items(): + if in_place != target_inplace: + continue last_bw = 0.0 - last_msg_size = act_res_dict[0]['size'] - for act_dict in act_res_dict: - if act_dict['inPlace'] == 1: - # Skip validation if this message size is not in reference - if str(act_dict['size']) not in ref_msg_sizes: - continue - - current_bw = float(act_dict['busBw']) - threshold = float(last_bw) * tolerance - if last_bw > 0 and current_bw < threshold: - fail_test( - f"The BusBW for msg size {act_dict['size']} = {current_bw} is less than the earlier msg size {last_msg_size} = BW {last_bw} (threshold with 5% tolerance: {threshold:.2f})" - ) - last_bw = act_dict['busBw'] - last_msg_size = act_dict['size'] + last_msg_size = None + for act_dict in rows: + # Skip validation if this message size is not in reference + if str(act_dict['size']) not in ref_msg_sizes: + continue + + current_bw = float(act_dict['busBw']) + threshold = float(last_bw) * tolerance + if last_bw > 0 and current_bw < threshold: + fail_test( + f"The BusBW for msg size {act_dict['size']} = {current_bw} (type={dtype}) is less than the " + f"earlier msg size {last_msg_size} = BW {last_bw} (threshold with 5% tolerance: {threshold:.2f})" + ) + last_bw = current_bw + last_msg_size = act_dict['size'] def check_lat_dip(test_name, output, exp_res_dict=None): @@ -435,13 +421,10 @@ def check_lat_dip(test_name, output, exp_res_dict=None): Check for latency decreases as message size increases (which would be unexpected). Only fails if latency drops by more than 5%. Only validates message sizes specified in the reference. If no reference provided, skips validation. - """ - # act_res_dict = json.loads(output.replace( '\n', '').replace( '\r', '')) - act_res_dict = output - tolerance = 0.95 # 5% tolerance - # Get reference message sizes if provided - # If no reference data, skip validation entirely + Rows are grouped by (data type, inPlace) and sorted ascending by message size + before comparison, for the same reason described in ``check_bw_dip``. + """ if not exp_res_dict: log.warning(f"No reference data provided for latency dip check, skipping validation for {test_name}") return @@ -449,60 +432,102 @@ def check_lat_dip(test_name, output, exp_res_dict=None): ref_msg_sizes = set(str(size) for size in exp_res_dict.keys()) log.info(f"Validating latency dip only for reference message sizes: {ref_msg_sizes}") - if re.search('alltoall|all_to_all', test_name, re.I): - last_time = 0.0 - last_msg_size = act_res_dict[0]['size'] - for act_dict in act_res_dict: - if act_dict['inPlace'] == 0: - # Skip validation if this message size is not in reference - if str(act_dict['size']) not in ref_msg_sizes: - continue - - current_time = float(act_dict['time']) - threshold = float(last_time) * tolerance - if last_time > 0 and current_time < threshold: - fail_test( - f"The latency for msg size {act_dict['size']} = {current_time} is less than the earlier msg size {last_msg_size} = latency {last_time} (threshold with 5% tolerance: {threshold:.2f})" - ) - last_time = act_dict['time'] - last_msg_size = act_dict['size'] - else: + target_inplace = 0 if re.search('alltoall|all_to_all', test_name, re.I) else 1 + tolerance = 0.95 # 5% tolerance + + for (dtype, in_place), rows in group_rccl_results(output).items(): + if in_place != target_inplace: + continue last_time = 0.0 - last_msg_size = act_res_dict[0]['size'] - for act_dict in act_res_dict: - if act_dict['inPlace'] == 1: - # Skip validation if this message size is not in reference - if str(act_dict['size']) not in ref_msg_sizes: - continue - - current_time = float(act_dict['time']) - threshold = float(last_time) * tolerance - if last_time > 0 and current_time < threshold: - fail_test( - f"The latency for msg size {act_dict['size']} = {current_time} is less than the earlier msg size {last_msg_size} = latency {last_time} (threshold with 5% tolerance: {threshold:.2f})" - ) - last_time = act_dict['time'] - last_msg_size = act_dict['size'] + last_msg_size = None + for act_dict in rows: + # Skip validation if this message size is not in reference + if str(act_dict['size']) not in ref_msg_sizes: + continue + + current_time = float(act_dict['time']) + threshold = float(last_time) * tolerance + if last_time > 0 and current_time < threshold: + fail_test( + f"The latency for msg size {act_dict['size']} = {current_time} (type={dtype}) is less than the " + f"earlier msg size {last_msg_size} = latency {last_time} (threshold with 5% tolerance: {threshold:.2f})" + ) + last_time = current_time + last_msg_size = act_dict['size'] + + +def _inplace_label(in_place): + """Human-readable label for an inPlace flag used in series names / logs.""" + if in_place == 1: + return 'in_place' + if in_place == 0: + return 'out_of_place' + return f'inPlace={in_place}' + + +def group_rccl_results(results): + """ + Group raw rccl-test rows by their full identity key ``(type, inPlace)`` and + return each group sorted ascending by message size. + + Why this exists: + A single rccl-test JSON result contains, for every message size, one row + per (data type, inPlace) combination. Any logic that walks the flat list + while tracking "previous" values (dip checks) or that buckets rows by + message size alone (graph/report building) will silently mix or overwrite + rows from different data types / in-place vs out-of-place measurements. + That both hides real regressions (overwritten rows disappear) and + manufactures fake ones (a dtype boundary looks like a giant bandwidth dip). + + Grouping on the full key and sorting within each group is the canonical + fix shared by every consumer so the behaviour is consistent. + + Args: + results (list[dict]): rccl-test rows, each with at least 'size', 'type', + and 'inPlace' keys. + + Returns: + dict[tuple, list[dict]]: mapping of (type, inPlace) -> rows sorted by size. + """ + groups = {} + for row in results: + key = (row.get('type', 'NA'), row.get('inPlace', 'NA')) + groups.setdefault(key, []).append(row) + for key in groups: + groups[key].sort(key=lambda r: int(r['size'])) + return groups def convert_to_graph_dict(result_dict): + """ + Convert raw per-series RCCL results into a graph-friendly nested dict. + + Each input series (keyed by '-') typically contains + multiple rows per message size: one per (data type, inPlace) combination. + The previous implementation keyed the output only by message size, so the + last row written for a size silently overwrote every earlier row - collapsing + in-place vs out-of-place (and multiple data types) into a single value and + hiding regressions in whichever dimension lost the race. + + To preserve every dimension we expand each (type, inPlace) pair into its own + output series. The inner ``{msg_size: {bus_bw, alg_bw, time}}`` mapping then + has exactly one row per size, which is what the HTML report builders expect. + """ graph_dict = {} for graph_series_name in result_dict.keys(): log.info("%s", graph_series_name) - graph_dict[graph_series_name] = {} dict_list = result_dict[graph_series_name] log.info("%s", dict_list) - for dict_item in dict_list: - msg_size = dict_item['size'] - graph_dict[graph_series_name][msg_size] = {} - if re.search('alltoall', dict_item['name'], re.I) and dict_item['inPlace'] == 1: - graph_dict[graph_series_name][msg_size]['bus_bw'] = dict_item['busBw'] - graph_dict[graph_series_name][msg_size]['alg_bw'] = dict_item['algBw'] - graph_dict[graph_series_name][msg_size]['time'] = dict_item['time'] - else: - graph_dict[graph_series_name][msg_size]['bus_bw'] = dict_item['busBw'] - graph_dict[graph_series_name][msg_size]['alg_bw'] = dict_item['algBw'] - graph_dict[graph_series_name][msg_size]['time'] = dict_item['time'] + for (dtype, in_place), rows in group_rccl_results(dict_list).items(): + series_key = f'{graph_series_name} type={dtype} {_inplace_label(in_place)}' + bucket = graph_dict.setdefault(series_key, {}) + for dict_item in rows: + msg_size = dict_item['size'] + bucket[msg_size] = { + 'bus_bw': dict_item['busBw'], + 'alg_bw': dict_item['algBw'], + 'time': dict_item['time'], + } log.info("%s", graph_dict) return graph_dict @@ -680,27 +705,42 @@ def rccl_regression( if output_algo_proto_channels: extra_flags += ' -A 1' + # Optional per-run library path (used by paired A/B testing to load a specific + # librccl.so build without rebuilding rccl-tests). Prepended to LD_LIBRARY_PATH + # inside the bash wrapper so it takes precedence for this run only. + ld_library_path = rccl_test_params.get('ld_library_path') + ld_prefix = f'export LD_LIBRARY_PATH={ld_library_path}:$LD_LIBRARY_PATH && ' if ld_library_path else '' + + # Optional data type (-d). When omitted the rccl-tests binary uses its default + # (float). Used by paired A/B testing to sweep multiple data types. + data_type = rccl_test_params.get('data_type') + dtype_flag = f' -d {data_type}' if data_type else '' + test_cmd = f'{rccl_tests_dir}/{test_name} -b {start_msg_size} -e {end_msg_size} -f {step_function} \ -t {threads_per_gpu} -w {warmup_iterations} -n {no_of_iterations} \ - -N {no_of_cycles} -c {check_iteration_count}{extra_flags} -Z json {output_flag} {rccl_result_file}' + -N {no_of_cycles} -c {check_iteration_count}{dtype_flag}{extra_flags} -Z json {output_flag} {rccl_result_file}' # Wrap with env file sourcing if env_file and str(env_file).lower() != 'none': - test_cmd = f'bash -c "source {env_file} && {test_cmd}"' + test_cmd = f'bash -c "source {env_file} && {ld_prefix}{test_cmd}"' else: # Always wrap in bash to interpret && shell operator - test_cmd = f'bash -c "{test_cmd}"' + test_cmd = f'bash -c "{ld_prefix}{test_cmd}"' # Build env override parameters for regression testing env_override_params = '' if env_overrides: env_override_params = ' '.join([f'-x {k}={v}' for k, v in env_overrides.items()]) - # Build mpirun command + # Build mpirun command. + # plm_rsh_args disables interactive host-key prompts so PRRTE can ssh-launch + # ranks on the other allocated nodes non-interactively (required for multi-node + # runs; harmless single-node). Mirrors the older working recipe. cmd = f'''{mpi_dir}/bin/mpirun \ --allow-run-as-root \ -np {no_of_global_ranks} \ --hostfile /tmp/rccl_hosts_file.txt \ + --mca plm_rsh_args "-o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null" \ --bind-to numa \ {ucx_params} \ --mca btl ^vader,openib \ diff --git a/cvs/lib/regression_lib.py b/cvs/lib/regression_lib.py new file mode 100644 index 000000000..ff22a12b5 --- /dev/null +++ b/cvs/lib/regression_lib.py @@ -0,0 +1,483 @@ +''' +Copyright 2025 Advanced Micro Devices, Inc. +All rights reserved. This notice is intended as a precaution against inadvertent publication and does not imply publication or any waiver of confidentiality. +The year included in the foregoing notice is the year of creation of the work. +All code contained here is Property of Advanced Micro Devices, Inc. +''' + +""" +Paired A/B regression detector for RCCL performance results. + +Motivation +---------- +Detecting RCCL performance regressions against a *static* baseline is unreliable, +especially for small messages (1 KiB .. a few MiB) where bus bandwidth is +latency-bound and has large run-to-run variation. A fixed threshold either +produces false positives (CI noise) or has to be set so loose that it hides real +regressions. + +Instead we run a *candidate* (B) and a *reference* (A) back-to-back on the same +nodes inside the same allocation, ideally interleaved over several repeats. Most +environmental noise (thermals, stragglers, NIC state, neighbour jobs) is then +common-mode and largely cancels in the paired comparison, so the difference +``A - B`` is far more stable than either absolute number. + +This module is intentionally pure-Python with no cluster / SSH / pandas +dependencies so it can be unit-tested exhaustively (including Monte-Carlo +false-positive sweeps) on a login node without GPUs. + +Design summary +-------------- +For every fully-qualified key ``(collective, size, type, inPlace)`` we collect a +sample of bandwidth measurements for side A and side B (one per repeat) and apply +THREE independent gates. A regression is only *confirmed* when all of them agree, +which is what makes the detector trustworthy in CI: + +1. Size-tiered relative threshold + - small (<= 1 MiB) : 20 % (very noisy, latency-bound) + - mid (<= 64 MiB) : 10 % + - large (> 64 MiB) : 5 % (bandwidth-bound, stable, regressions matter) + B must be slower than A by MORE than the tier threshold (median vs median). + +2. Non-parametric separation gate + B's upper quartile must sit below A's lower quartile (``p75(B) < p25(A)``), + i.e. the two distributions barely overlap. This is robust to single-run + outliers / stragglers and needs no distributional assumptions. + +3. Adjacency confirmation + A real regression usually spans a contiguous band of message sizes, whereas + noise tends to be isolated. A candidate size is only confirmed if it belongs + to a run of >= ``adjacency_min_run`` consecutive candidate sizes within the + same ``(collective, type, inPlace)`` group. + +Additional safety: keys whose reference bandwidth is below ``min_bandwidth_floor`` +or that have fewer than ``min_repeats`` samples per side are reported as +INCONCLUSIVE (never as a regression). +""" + +import statistics +from copy import deepcopy + +# Verdict constants +PASS = "pass" +REGRESSION = "regression" +INCONCLUSIVE = "inconclusive" + +KiB = 1024 +MiB = 1024 * 1024 + +DEFAULT_CONFIG = { + # Metric to compare and its direction. For bandwidth higher is better; a + # regression means B < A. (Set "higher_is_better": False for latency-like + # metrics, where a regression means B > A.) + "metric": "busBw", + "higher_is_better": True, + + # Relative regression thresholds per size tier (fraction of A). + "thresholds": { + "small": 0.20, + "mid": 0.10, + "large": 0.05, + }, + # Inclusive upper byte boundaries for the small / mid tiers. + "tier_boundaries": { + "small_max_bytes": 1 * MiB, + "mid_max_bytes": 64 * MiB, + }, + + # Non-parametric separation gate. + "separation_gate": True, + "separation_b_percentile": 75, + "separation_a_percentile": 25, + + # Adjacency confirmation. Set to 1 to disable (flag isolated sizes too). + "adjacency_min_run": 2, + + # Keys whose reference (A) median metric is below this floor are skipped + # (relative deltas explode near zero). Units match the metric (GB/s). + "min_bandwidth_floor": 0.5, + + # Minimum repeats per side; below this a key is INCONCLUSIVE. + "min_repeats": 2, +} + + +def merge_config(overrides=None): + """Return DEFAULT_CONFIG deep-merged with ``overrides`` (one level deep on dicts).""" + cfg = deepcopy(DEFAULT_CONFIG) + if not overrides: + return cfg + for key, value in overrides.items(): + if isinstance(value, dict) and isinstance(cfg.get(key), dict): + cfg[key] = {**cfg[key], **value} + else: + cfg[key] = value + return cfg + + +def percentile(samples, pct): + """ + Linear-interpolation percentile (numpy 'linear' method) without numpy. + + Args: + samples: non-empty iterable of numbers. + pct: percentile in [0, 100]. + + Returns: + float percentile value. + """ + data = sorted(float(s) for s in samples) + if not data: + raise ValueError("percentile() requires at least one sample") + if len(data) == 1: + return data[0] + rank = (pct / 100.0) * (len(data) - 1) + low = int(rank) + high = min(low + 1, len(data) - 1) + frac = rank - low + return data[low] + (data[high] - data[low]) * frac + + +def median(samples): + """Median via the 50th percentile helper.""" + return percentile(samples, 50) + + +def summarize_samples(samples): + """Return robust summary statistics for a list of samples.""" + data = [float(s) for s in samples] + return { + "n": len(data), + "min": min(data), + "max": max(data), + "median": median(data), + "mean": sum(data) / len(data), + "p25": percentile(data, 25), + "p75": percentile(data, 75), + } + + +def size_tier(size_bytes, config=None): + """Classify a message size into 'small' | 'mid' | 'large'.""" + cfg = config or DEFAULT_CONFIG + bounds = cfg["tier_boundaries"] + size_bytes = int(size_bytes) + if size_bytes <= bounds["small_max_bytes"]: + return "small" + if size_bytes <= bounds["mid_max_bytes"]: + return "mid" + return "large" + + +def threshold_for(size_bytes, config=None): + """Return the relative regression threshold (fraction) for a message size.""" + cfg = config or DEFAULT_CONFIG + return cfg["thresholds"][size_tier(size_bytes, cfg)] + + +def _group_runs_by_key(runs, metric): + """ + Flatten a list of runs (each a list of rccl rows) into a mapping of + ``(name, size, type, inPlace) -> [metric samples]``. + + Rows missing the metric or any key field are ignored. + """ + samples = {} + for run in runs: + for row in run: + try: + key = ( + row["name"], + int(row["size"]), + row.get("type", "NA"), + row.get("inPlace", "NA"), + ) + value = float(row[metric]) + except (KeyError, TypeError, ValueError): + continue + samples.setdefault(key, []).append(value) + return samples + + +def _relative_drop(a_value, b_value, higher_is_better): + """ + Relative regression magnitude (fraction). Positive means B is worse than A. + + For higher-is-better metrics this is ``(A - B) / A``; for lower-is-better + metrics it is ``(B - A) / A``. + """ + if a_value == 0: + return 0.0 + if higher_is_better: + return (a_value - b_value) / a_value + return (b_value - a_value) / a_value + + +def compare_key(a_samples, b_samples, size_bytes, config=None): + """ + Evaluate a single fully-qualified key and return a candidate verdict dict. + + The returned verdict is a *candidate* only (threshold + separation gates). + Adjacency confirmation is applied later by ``detect_regressions`` because it + needs the neighbouring sizes. + """ + cfg = merge_config(config) + metric = cfg["metric"] + hib = cfg["higher_is_better"] + + a_stats = summarize_samples(a_samples) + b_stats = summarize_samples(b_samples) + + result = { + "metric": metric, + "size": int(size_bytes), + "tier": size_tier(size_bytes, cfg), + "threshold": threshold_for(size_bytes, cfg), + "a": a_stats, + "b": b_stats, + "rel_drop": _relative_drop(a_stats["median"], b_stats["median"], hib), + "candidate": False, + "verdict": PASS, + "reasons": [], + } + + # Guard: insufficient repeats. + if a_stats["n"] < cfg["min_repeats"] or b_stats["n"] < cfg["min_repeats"]: + result["verdict"] = INCONCLUSIVE + result["reasons"].append( + f"insufficient repeats (A={a_stats['n']}, B={b_stats['n']}, need {cfg['min_repeats']})" + ) + return result + + # Guard: reference too small to compare reliably. + if a_stats["median"] < cfg["min_bandwidth_floor"]: + result["verdict"] = INCONCLUSIVE + result["reasons"].append( + f"reference median {a_stats['median']:.3f} below floor {cfg['min_bandwidth_floor']}" + ) + return result + + # Gate 1: size-tiered relative threshold. + passed_threshold = result["rel_drop"] > result["threshold"] + + # Gate 2: non-parametric separation. + if cfg["separation_gate"]: + b_hi = percentile(b_samples, cfg["separation_b_percentile"]) + a_lo = percentile(a_samples, cfg["separation_a_percentile"]) + if hib: + # regression => B clearly below A + passed_separation = b_hi < a_lo + else: + # regression => B clearly above A + b_lo = percentile(b_samples, 100 - cfg["separation_b_percentile"]) + a_hi = percentile(a_samples, 100 - cfg["separation_a_percentile"]) + passed_separation = b_lo > a_hi + result["separation"] = {"b_edge": b_hi if hib else b_lo, "a_edge": a_lo if hib else a_hi} + else: + passed_separation = True + + if passed_threshold and passed_separation: + result["candidate"] = True + else: + if not passed_threshold: + result["reasons"].append( + f"rel_drop {result['rel_drop']:.3f} <= threshold {result['threshold']:.3f}" + ) + if not passed_separation: + result["reasons"].append("distributions overlap (separation gate not met)") + return result + + +def detect_regressions(a_runs, b_runs, config=None): + """ + Run the full paired A/B regression analysis. + + Args: + a_runs: list of reference runs. Each run is a list of rccl-test rows + (dicts with 'name', 'size', 'type', 'inPlace' and the configured metric). + b_runs: list of candidate runs in the same row format. + config: optional dict of overrides for DEFAULT_CONFIG. + + Returns: + dict report: + { + "config": , + "summary": {"keys_compared", "regressions", "inconclusive", + "candidates", "has_regression"}, + "keys": [ per-key verdict dicts, with 'confirmed' set ], + "regressions": [ confirmed regression verdicts ], + } + """ + cfg = merge_config(config) + metric = cfg["metric"] + + a_samples = _group_runs_by_key(a_runs, metric) + b_samples = _group_runs_by_key(b_runs, metric) + + common_keys = set(a_samples) & set(b_samples) + + # Evaluate each common key for candidacy. + per_key = {} + for key in common_keys: + name, size, dtype, in_place = key + verdict = compare_key(a_samples[key], b_samples[key], size, cfg) + verdict["key"] = {"name": name, "size": size, "type": dtype, "inPlace": in_place} + verdict["confirmed"] = False + per_key[key] = verdict + + # Adjacency confirmation: within each (name, type, inPlace) group, sort by + # size and confirm candidates that belong to a run of >= adjacency_min_run + # consecutive candidate sizes. + min_run = max(1, int(cfg["adjacency_min_run"])) + groups = {} + for key in per_key: + name, size, dtype, in_place = key + groups.setdefault((name, dtype, in_place), []).append(key) + + for group_keys in groups.values(): + group_keys.sort(key=lambda k: k[1]) # by size + run_start = 0 + n = len(group_keys) + i = 0 + while i < n: + if per_key[group_keys[i]]["candidate"]: + j = i + while j < n and per_key[group_keys[j]]["candidate"]: + j += 1 + run_len = j - i + if run_len >= min_run: + for k in range(i, j): + per_key[group_keys[k]]["confirmed"] = True + per_key[group_keys[k]]["verdict"] = REGRESSION + else: + for k in range(i, j): + per_key[group_keys[k]]["reasons"].append( + f"isolated candidate (run length {run_len} < adjacency_min_run {min_run})" + ) + i = j + else: + i += 1 + _ = run_start # silence unused + + keys_list = [per_key[k] for k in sorted(per_key, key=lambda k: (k[0], k[2], k[3], k[1]))] + regressions = [v for v in keys_list if v["verdict"] == REGRESSION] + inconclusive = [v for v in keys_list if v["verdict"] == INCONCLUSIVE] + candidates = [v for v in keys_list if v["candidate"]] + + report = { + "config": cfg, + "summary": { + "keys_compared": len(keys_list), + "regressions": len(regressions), + "inconclusive": len(inconclusive), + "candidates": len(candidates), + "has_regression": len(regressions) > 0, + }, + "keys": keys_list, + "regressions": regressions, + } + return report + + +def measure_noise(control_runs, config=None): + """ + Measure per-tier run-to-run noise from a *control* dataset. + + A control dataset is produced by running the SAME build as both sides (A=B), + so any spread across repeats is pure run-to-run / environmental noise. This + is the empirical noise floor used to choose trustworthy thresholds. + + Args: + control_runs: list of runs (each a list of rccl rows) from one build. + config: optional overrides (uses 'metric' and 'min_bandwidth_floor'). + + Returns: + dict {tier: {'n_keys', 'cv_median', 'cv_p95', 'rel_range_p95'} or None} + where cv is the coefficient of variation (stdev/median) per key. + """ + cfg = merge_config(config) + samples = _group_runs_by_key(control_runs, cfg["metric"]) + per_tier = {"small": [], "mid": [], "large": []} + for (name, size, dtype, in_place), vals in samples.items(): + if len(vals) < 2: + continue + med = median(vals) + if med < cfg["min_bandwidth_floor"]: + continue + cv = statistics.pstdev(vals) / med if med else 0.0 + rel_range = (max(vals) - min(vals)) / med if med else 0.0 + per_tier[size_tier(size, cfg)].append((cv, rel_range)) + + out = {} + for tier, lst in per_tier.items(): + if not lst: + out[tier] = None + continue + cvs = [x[0] for x in lst] + ranges = [x[1] for x in lst] + out[tier] = { + "n_keys": len(lst), + "cv_median": percentile(cvs, 50), + "cv_p95": percentile(cvs, 95), + "rel_range_p95": percentile(ranges, 95), + } + return out + + +def derive_thresholds(control_runs, config=None, safety_factor=2.0, min_thresholds=None): + """ + Recommend per-tier regression thresholds from a control (A=B) dataset. + + The recommended threshold for a tier is ``safety_factor * p95(CV)`` of that + tier's measured run-to-run noise, clamped to a sensible minimum. Sitting the + threshold a couple of noise-widths above the observed spread is what keeps + the detector from firing on noise while still catching real shifts. + + Args: + control_runs: control dataset (same build run repeatedly). + config: optional config overrides. + safety_factor: multiple of the p95 noise CV to use as the threshold. + min_thresholds: per-tier floors; defaults to small=0.10, mid=0.05, large=0.03. + + Returns: + dict {'thresholds': {tier: value}, 'noise': , + 'safety_factor': ...} + """ + cfg = merge_config(config) + noise = measure_noise(control_runs, cfg) + mins = min_thresholds or {"small": 0.10, "mid": 0.05, "large": 0.03} + thresholds = {} + for tier in ("small", "mid", "large"): + tier_noise = noise.get(tier) + base = tier_noise["cv_p95"] * safety_factor if tier_noise else mins[tier] + thresholds[tier] = round(max(base, mins[tier]), 3) + return {"thresholds": thresholds, "noise": noise, "safety_factor": safety_factor} + + +def format_report(report, max_rows=50): + """Render a compact human-readable summary of a detect_regressions() report.""" + s = report["summary"] + lines = [] + lines.append("==================== RCCL A/B Regression Report ====================") + lines.append( + f"keys compared : {s['keys_compared']} " + f"confirmed regressions : {s['regressions']} " + f"inconclusive : {s['inconclusive']}" + ) + lines.append(f"verdict : {'REGRESSION DETECTED' if s['has_regression'] else 'PASS'}") + if report["regressions"]: + lines.append("") + lines.append("Confirmed regressions:") + lines.append( + f" {'collective':<20} {'type':<10} {'inPl':>4} {'size':>12} " + f"{'A_med':>10} {'B_med':>10} {'drop%':>7} {'thr%':>6}" + ) + for v in report["regressions"][:max_rows]: + k = v["key"] + lines.append( + f" {k['name']:<20} {str(k['type']):<10} {str(k['inPlace']):>4} {k['size']:>12} " + f"{v['a']['median']:>10.2f} {v['b']['median']:>10.2f} " + f"{v['rel_drop'] * 100:>6.1f}% {v['threshold'] * 100:>5.1f}%" + ) + lines.append("====================================================================") + return "\n".join(lines) diff --git a/cvs/lib/unittests/test_rccl_lib.py b/cvs/lib/unittests/test_rccl_lib.py index 3dbc3c05e..d06fcbf8e 100644 --- a/cvs/lib/unittests/test_rccl_lib.py +++ b/cvs/lib/unittests/test_rccl_lib.py @@ -270,6 +270,118 @@ def test_check_lat_dip_no_reference(self, mock_fail_test): rccl_lib.check_lat_dip(test_name, output, None) mock_fail_test.assert_not_called() + # --------------------------------------------------------------------- + # Group-by / key-correctness regression tests (AIMVT-196) + # --------------------------------------------------------------------- + + def test_group_rccl_results_groups_and_sorts(self): + """Rows are grouped by (type, inPlace) and sorted ascending by size.""" + rows = [ + {"size": 2048, "type": "float", "inPlace": 1, "busBw": 20.0}, + {"size": 1024, "type": "float", "inPlace": 1, "busBw": 10.0}, + {"size": 1024, "type": "float", "inPlace": 0, "busBw": 9.0}, + {"size": 1024, "type": "bfloat16", "inPlace": 1, "busBw": 11.0}, + ] + groups = rccl_lib.group_rccl_results(rows) + # Three distinct (type, inPlace) groups + self.assertEqual( + set(groups.keys()), + {("float", 1), ("float", 0), ("bfloat16", 1)}, + ) + # float/in-place group sorted ascending by size + sizes = [r["size"] for r in groups[("float", 1)]] + self.assertEqual(sizes, [1024, 2048]) + + def test_convert_to_graph_dict_preserves_inplace(self): + """In-place and out-of-place rows for the same size must NOT collapse.""" + result_dict = { + "all_reduce_perf-NCCL_ALGO=Ring": [ + {"size": 1024, "name": "AllReduce", "type": "float", "inPlace": 0, + "busBw": 10.0, "algBw": 5.0, "time": 1.0}, + {"size": 1024, "name": "AllReduce", "type": "float", "inPlace": 1, + "busBw": 99.0, "algBw": 50.0, "time": 2.0}, + ] + } + graph = rccl_lib.convert_to_graph_dict(result_dict) + # Two separate series, one per inPlace orientation + self.assertEqual(len(graph), 2) + in_series = next(k for k in graph if "in_place" in k) + out_series = next(k for k in graph if "out_of_place" in k) + self.assertEqual(graph[out_series][1024]["bus_bw"], 10.0) + self.assertEqual(graph[in_series][1024]["bus_bw"], 99.0) + + def test_convert_to_graph_dict_preserves_dtype(self): + """Different data types for the same size must NOT collapse.""" + result_dict = { + "all_reduce_perf-NCCL_ALGO=Ring": [ + {"size": 1024, "name": "AllReduce", "type": "float", "inPlace": 1, + "busBw": 10.0, "algBw": 5.0, "time": 1.0}, + {"size": 1024, "name": "AllReduce", "type": "bfloat16", "inPlace": 1, + "busBw": 20.0, "algBw": 10.0, "time": 1.0}, + ] + } + graph = rccl_lib.convert_to_graph_dict(result_dict) + self.assertEqual(len(graph), 2) + float_series = next(k for k in graph if "type=float" in k) + bf16_series = next(k for k in graph if "type=bfloat16" in k) + self.assertEqual(graph[float_series][1024]["bus_bw"], 10.0) + self.assertEqual(graph[bf16_series][1024]["bus_bw"], 20.0) + + @patch('cvs.lib.rccl_lib.fail_test') + def test_check_bw_dip_multi_dtype_no_false_positive(self, mock_fail_test): + """A data-type boundary must not be mistaken for a bandwidth dip.""" + test_name = "all_reduce_perf" + # float ascending then bfloat16 restarting at the smallest size. + output = [ + {"size": 1024, "type": "float", "inPlace": 1, "busBw": 10.0, "time": 1.0}, + {"size": 2048, "type": "float", "inPlace": 1, "busBw": 20.0, "time": 2.0}, + {"size": 1024, "type": "bfloat16", "inPlace": 1, "busBw": 10.0, "time": 1.0}, + {"size": 2048, "type": "bfloat16", "inPlace": 1, "busBw": 20.0, "time": 2.0}, + ] + ref = {"1024": {"bus_bw": 1}, "2048": {"bus_bw": 1}} + rccl_lib.check_bw_dip(test_name, output, ref) + mock_fail_test.assert_not_called() + + @patch('cvs.lib.rccl_lib.fail_test') + def test_check_bw_dip_detects_real_dip_within_dtype(self, mock_fail_test): + """A genuine within-data-type bandwidth dip is still flagged.""" + test_name = "all_reduce_perf" + output = [ + {"size": 1024, "type": "float", "inPlace": 1, "busBw": 100.0, "time": 1.0}, + {"size": 2048, "type": "float", "inPlace": 1, "busBw": 50.0, "time": 2.0}, + ] + ref = {"1024": {"bus_bw": 1}, "2048": {"bus_bw": 1}} + rccl_lib.check_bw_dip(test_name, output, ref) + mock_fail_test.assert_called() + + @patch('cvs.lib.rccl_lib.fail_test') + def test_check_lat_dip_multi_dtype_no_false_positive(self, mock_fail_test): + """A data-type boundary must not be mistaken for a latency dip.""" + test_name = "all_reduce_perf" + output = [ + {"size": 1024, "type": "float", "inPlace": 1, "busBw": 10.0, "time": 10.0}, + {"size": 2048, "type": "float", "inPlace": 1, "busBw": 20.0, "time": 20.0}, + {"size": 1024, "type": "bfloat16", "inPlace": 1, "busBw": 10.0, "time": 10.0}, + {"size": 2048, "type": "bfloat16", "inPlace": 1, "busBw": 20.0, "time": 20.0}, + ] + ref = {"1024": {"bus_bw": 1}, "2048": {"bus_bw": 1}} + rccl_lib.check_lat_dip(test_name, output, ref) + mock_fail_test.assert_not_called() + + @patch('cvs.lib.rccl_lib.fail_test') + def test_check_bus_bw_multi_dtype_each_compared(self, mock_fail_test): + """Both data types are compared against the size-keyed reference.""" + test_name = "all_reduce_perf" + output = [ + {"name": "all_reduce_perf", "size": 1024, "type": "float", "inPlace": 1, + "busBw": 90.0, "algBw": 45.0, "time": 12.3}, + {"name": "all_reduce_perf", "size": 1024, "type": "bfloat16", "inPlace": 1, + "busBw": 70.0, "algBw": 35.0, "time": 12.3}, # below threshold -> must fail + ] + exp_res_dict = {"1024": {"bus_bw": 80.0}} # 95% threshold = 76.0 + rccl_lib.check_bus_bw(test_name, output, exp_res_dict) + mock_fail_test.assert_called() + if __name__ == '__main__': unittest.main() diff --git a/cvs/lib/unittests/test_regression_lib.py b/cvs/lib/unittests/test_regression_lib.py new file mode 100644 index 000000000..f5d85d774 --- /dev/null +++ b/cvs/lib/unittests/test_regression_lib.py @@ -0,0 +1,274 @@ +# cvs/lib/unittests/test_regression_lib.py +""" +Unit tests for the paired A/B regression detector (cvs.lib.regression_lib). + +Includes deterministic correctness tests plus Monte-Carlo sweeps that verify the +detector's two most important properties for CI: + * running a candidate against an identical-distribution reference produces an + extremely low false-positive rate (trustworthy / no flaky failures), and + * a genuine regression larger than the size-tier threshold is reliably caught. +""" + +import math +import random +import unittest + +import cvs.lib.regression_lib as reg + +KiB = 1024 +MiB = 1024 * 1024 +GiB = 1024 * 1024 * 1024 + +# Message size sweep used by the simulations: 1 KiB .. 4 GiB, powers of two. +SWEEP_SIZES = [1 << e for e in range(10, 33)] # 2^10 (1KiB) .. 2^32 (4GiB) + + +def _true_bw(size): + """A plausible bandwidth curve: latency-bound small, plateau at large size.""" + plateau = 350.0 # GB/s + half = 16 * MiB + return plateau * size / (size + half) + + +def _cv_for_size(size): + """Run-to-run coefficient of variation: noisy small, tight large.""" + if size <= 1 * MiB: + return 0.12 + if size <= 64 * MiB: + return 0.05 + return 0.025 + + +def _make_run(rng, collective="AllReduce", dtype="float", in_place=1, bw_scale=1.0, sizes=None): + """Build one simulated sweep (list of rccl-style rows) with Gaussian noise.""" + rows = [] + for size in (sizes or SWEEP_SIZES): + cv = _cv_for_size(size) + mean = _true_bw(size) * bw_scale + val = mean * (1.0 + rng.gauss(0.0, cv)) + val = max(val, 0.01) + rows.append({ + "name": collective, + "size": size, + "type": dtype, + "inPlace": in_place, + "busBw": val, + "algBw": val * 0.5, + "time": 1.0, + }) + return rows + + +class TestHelpers(unittest.TestCase): + def test_percentile_basic(self): + data = [1, 2, 3, 4, 5] + self.assertEqual(reg.percentile(data, 0), 1) + self.assertEqual(reg.percentile(data, 100), 5) + self.assertEqual(reg.percentile(data, 50), 3) + self.assertEqual(reg.median(data), 3) + + def test_percentile_single(self): + self.assertEqual(reg.percentile([42], 25), 42) + + def test_summarize(self): + s = reg.summarize_samples([10, 20, 30]) + self.assertEqual(s["n"], 3) + self.assertEqual(s["min"], 10) + self.assertEqual(s["max"], 30) + self.assertEqual(s["median"], 20) + self.assertAlmostEqual(s["mean"], 20.0) + + def test_size_tier_and_threshold(self): + self.assertEqual(reg.size_tier(1 * KiB), "small") + self.assertEqual(reg.size_tier(1 * MiB), "small") + self.assertEqual(reg.size_tier(2 * MiB), "mid") + self.assertEqual(reg.size_tier(64 * MiB), "mid") + self.assertEqual(reg.size_tier(128 * MiB), "large") + self.assertEqual(reg.size_tier(4 * GiB), "large") + self.assertEqual(reg.threshold_for(1 * KiB), 0.20) + self.assertEqual(reg.threshold_for(2 * MiB), 0.10) + self.assertEqual(reg.threshold_for(1 * GiB), 0.05) + + def test_merge_config_overrides(self): + cfg = reg.merge_config({"thresholds": {"large": 0.08}, "adjacency_min_run": 3}) + self.assertEqual(cfg["thresholds"]["large"], 0.08) + self.assertEqual(cfg["thresholds"]["small"], 0.20) # untouched + self.assertEqual(cfg["adjacency_min_run"], 3) + + +class TestCompareKey(unittest.TestCase): + def test_clear_regression_is_candidate(self): + a = [100.0, 101.0, 99.0, 100.5, 100.0] + b = [80.0, 81.0, 79.0, 80.5, 80.0] # ~20% lower, tight + v = reg.compare_key(a, b, size_bytes=1 * GiB) # large tier, 5% thr + self.assertTrue(v["candidate"]) + self.assertGreater(v["rel_drop"], 0.05) + + def test_noise_only_not_candidate(self): + a = [100.0, 102.0, 98.0, 101.0, 99.0] + b = [101.0, 99.0, 100.0, 98.0, 102.0] # same distribution + v = reg.compare_key(a, b, size_bytes=1 * GiB) + self.assertFalse(v["candidate"]) + + def test_insufficient_repeats_inconclusive(self): + v = reg.compare_key([100.0], [50.0], size_bytes=1 * GiB) + self.assertEqual(v["verdict"], reg.INCONCLUSIVE) + + def test_below_floor_inconclusive(self): + v = reg.compare_key([0.1, 0.1, 0.1], [0.01, 0.01, 0.01], size_bytes=1 * KiB, + config={"min_bandwidth_floor": 0.5}) + self.assertEqual(v["verdict"], reg.INCONCLUSIVE) + + def test_separation_gate_blocks_overlap(self): + # Median drop (10%) exceeds the 5% large-tier threshold, but B is highly + # variable and overlaps A, so the separation gate must veto the candidate. + a = [100.0, 100.0, 100.0, 100.0, 100.0] + b = [70.0, 85.0, 90.0, 110.0, 115.0] # median 90, but p75(B)=110 >= p25(A)=100 + v = reg.compare_key(a, b, size_bytes=1 * GiB) + self.assertGreater(v["rel_drop"], 0.05) # threshold gate alone would pass + self.assertFalse(v["candidate"]) # separation gate vetoes it + + +class TestDetectRegressions(unittest.TestCase): + def test_identical_runs_no_regression(self): + rng = random.Random(0) + runs = [_make_run(rng) for _ in range(5)] + # Compare the exact same runs A vs A. + report = reg.detect_regressions(runs, runs) + self.assertFalse(report["summary"]["has_regression"]) + self.assertEqual(report["summary"]["regressions"], 0) + + def test_real_regression_band_confirmed(self): + rng = random.Random(1) + a_runs = [_make_run(rng, bw_scale=1.0) for _ in range(5)] + # B is 15% slower across ALL sizes -> large tier (5%/10%) easily flagged, + # and the regression spans many adjacent sizes so adjacency confirms. + b_runs = [_make_run(rng, bw_scale=0.85) for _ in range(5)] + report = reg.detect_regressions(a_runs, b_runs) + self.assertTrue(report["summary"]["has_regression"]) + # Large-tier sizes must be among the confirmed regressions. + big = [r for r in report["regressions"] if r["key"]["size"] >= 128 * MiB] + self.assertTrue(len(big) >= 2) + + def test_isolated_candidate_not_confirmed(self): + # Construct A and B identical except one isolated large size dropped hard. + sizes = [256 * MiB, 512 * MiB, 1 * GiB, 2 * GiB, 4 * GiB] + a_runs = [] + b_runs = [] + for _ in range(5): + a_rows = [{"name": "AllReduce", "size": s, "type": "float", "inPlace": 1, + "busBw": 300.0, "algBw": 150.0, "time": 1.0} for s in sizes] + b_rows = [] + for s in sizes: + bw = 300.0 + if s == 1 * GiB: # single isolated size regressed 30% + bw = 210.0 + b_rows.append({"name": "AllReduce", "size": s, "type": "float", "inPlace": 1, + "busBw": bw, "algBw": bw / 2, "time": 1.0}) + a_runs.append(a_rows) + b_runs.append(b_rows) + report = reg.detect_regressions(a_runs, b_runs, config={"adjacency_min_run": 2}) + # The single isolated size is a candidate but must NOT be confirmed. + self.assertFalse(report["summary"]["has_regression"]) + self.assertEqual(report["summary"]["candidates"], 1) + + def test_isolated_candidate_confirmed_when_adjacency_disabled(self): + sizes = [1 * GiB, 2 * GiB] + a_runs = [[{"name": "AllReduce", "size": 1 * GiB, "type": "float", "inPlace": 1, + "busBw": 300.0, "algBw": 150.0, "time": 1.0}, + {"name": "AllReduce", "size": 2 * GiB, "type": "float", "inPlace": 1, + "busBw": 300.0, "algBw": 150.0, "time": 1.0}] for _ in range(5)] + b_runs = [[{"name": "AllReduce", "size": 1 * GiB, "type": "float", "inPlace": 1, + "busBw": 210.0, "algBw": 105.0, "time": 1.0}, + {"name": "AllReduce", "size": 2 * GiB, "type": "float", "inPlace": 1, + "busBw": 300.0, "algBw": 150.0, "time": 1.0}] for _ in range(5)] + report = reg.detect_regressions(a_runs, b_runs, config={"adjacency_min_run": 1}) + self.assertTrue(report["summary"]["has_regression"]) + + def test_small_message_regression_below_threshold_skipped(self): + # A 12% drop at a small (noisy) size is below the 20% small-tier threshold. + size = 4 * KiB + a_runs = [[{"name": "AllReduce", "size": size, "type": "float", "inPlace": 1, + "busBw": 10.0, "algBw": 5.0, "time": 1.0}] for _ in range(5)] + b_runs = [[{"name": "AllReduce", "size": size, "type": "float", "inPlace": 1, + "busBw": 8.8, "algBw": 4.4, "time": 1.0}] for _ in range(5)] + report = reg.detect_regressions(a_runs, b_runs) + self.assertFalse(report["summary"]["has_regression"]) + + +class TestMonteCarlo(unittest.TestCase): + """Empirical stability checks with realistic noise.""" + + N_TRIALS = 200 + REPEATS = 5 + + def test_false_positive_rate_is_tiny(self): + """A vs A (same distribution): confirmed regressions should be ~never.""" + rng = random.Random(12345) + false_positives = 0 + for _ in range(self.N_TRIALS): + a_runs = [_make_run(rng, bw_scale=1.0) for _ in range(self.REPEATS)] + b_runs = [_make_run(rng, bw_scale=1.0) for _ in range(self.REPEATS)] + report = reg.detect_regressions(a_runs, b_runs) + if report["summary"]["has_regression"]: + false_positives += 1 + fp_rate = false_positives / self.N_TRIALS + # Triple-gated detector should essentially never false-positive. + self.assertLessEqual(fp_rate, 0.01, f"false-positive rate too high: {fp_rate:.3f}") + + def test_detection_rate_for_real_regression(self): + """A 15% uniform slowdown should be caught nearly every time.""" + rng = random.Random(999) + detected = 0 + for _ in range(self.N_TRIALS): + a_runs = [_make_run(rng, bw_scale=1.0) for _ in range(self.REPEATS)] + b_runs = [_make_run(rng, bw_scale=0.85) for _ in range(self.REPEATS)] + report = reg.detect_regressions(a_runs, b_runs) + if report["summary"]["has_regression"]: + detected += 1 + detect_rate = detected / self.N_TRIALS + self.assertGreaterEqual(detect_rate, 0.95, f"detection rate too low: {detect_rate:.3f}") + + +class TestThresholdDerivation(unittest.TestCase): + def test_measure_noise_reflects_input_cv(self): + """measured CV should be in the right ballpark for each tier's noise.""" + rng = random.Random(7) + # Many repeats of the same build -> control dataset. + control = [_make_run(rng, bw_scale=1.0) for _ in range(20)] + noise = reg.measure_noise(control) + # large tier noise (cv ~0.025) should be well below small tier (cv ~0.12) + self.assertIsNotNone(noise["large"]) + self.assertIsNotNone(noise["mid"]) + self.assertLess(noise["large"]["cv_p95"], 0.08) + self.assertGreater(noise["mid"]["cv_p95"], noise["large"]["cv_p95"]) + + def test_derive_thresholds_above_noise(self): + rng = random.Random(8) + control = [_make_run(rng, bw_scale=1.0) for _ in range(20)] + derived = reg.derive_thresholds(control, safety_factor=2.0) + th = derived["thresholds"] + # Each derived threshold must sit above the measured p95 noise for the tier. + for tier in ("mid", "large"): + if derived["noise"][tier]: + self.assertGreaterEqual(th[tier], derived["noise"][tier]["cv_p95"]) + # Thresholds respect the configured minimums. + self.assertGreaterEqual(th["large"], 0.03) + + def test_derived_thresholds_give_zero_false_positives(self): + """End-to-end: thresholds derived from control data => no A/B false positives.""" + rng = random.Random(101) + control = [_make_run(rng, bw_scale=1.0) for _ in range(15)] + derived = reg.derive_thresholds(control, safety_factor=2.0) + cfg = {"thresholds": derived["thresholds"]} + fp = 0 + for _ in range(150): + a = [_make_run(rng, bw_scale=1.0) for _ in range(7)] + b = [_make_run(rng, bw_scale=1.0) for _ in range(7)] + if reg.detect_regressions(a, b, config=cfg)["summary"]["has_regression"]: + fp += 1 + self.assertEqual(fp, 0, f"derived thresholds produced {fp} false positives") + + +if __name__ == "__main__": + unittest.main() diff --git a/cvs/tests/rccl/rccl_ab_regression.py b/cvs/tests/rccl/rccl_ab_regression.py new file mode 100644 index 000000000..de414b6a0 --- /dev/null +++ b/cvs/tests/rccl/rccl_ab_regression.py @@ -0,0 +1,315 @@ +''' +Copyright 2025 Advanced Micro Devices, Inc. +All rights reserved. This notice is intended as a precaution against inadvertent publication and does not imply publication or any waiver of confidentiality. +The year included in the foregoing notice is the year of creation of the work. +All code contained here is Property of Advanced Micro Devices, Inc. +''' + +""" +Paired A/B RCCL regression test. + +For each (collective, data type, NCCL env-combo) this runs a reference build (A) +and a candidate build (B) back-to-back, interleaved, for a configurable number of +repeats. Because both sides run on the same nodes within the same allocation, +environmental noise is largely common-mode and cancels in the paired comparison. + +All pass/fail logic lives in ``cvs.lib.regression_lib`` (pure, unit-tested). This +module is only the cluster orchestration: it produces the A and B result samples +and hands them to the detector. + +Modes (config: rccl.ab_regression): + - control_mode=true : both sides use the reference build (A=B). Used to measure + the on-hardware run-to-run noise floor, derive thresholds, and prove the + detector reports zero regressions on an identical build. + - control_mode=false: reference build vs candidate build (real detection). + +See cvs/input/config_file/rccl/rccl_ab_config.json.sample for the config shape. +""" + +import os +import json +import itertools +import copy + +import pytest + +from cvs.lib import rccl_lib +from cvs.lib import regression_lib +from cvs.lib.parallel_ssh_lib import * +from cvs.lib.utils_lib import * +from cvs.lib.verify_lib import * +from cvs.lib import globals + +log = globals.log + + +# Accumulates per-(collective, dtype, env-combo) A and B run samples across the +# parametrized test invocations, consumed by the final analysis test. +# ab_runs[group_key] = {"a": [run, run, ...], "b": [run, run, ...]} +ab_runs = {} + + +# --------------------------------------------------------------------------- # +# Fixtures (mirror cvs/tests/rccl/rccl_regression.py) +# --------------------------------------------------------------------------- # +@pytest.fixture(scope="module") +def cluster_file(pytestconfig): + return pytestconfig.getoption("cluster_file") + + +@pytest.fixture(scope="module") +def config_file(pytestconfig): + return pytestconfig.getoption("config_file") + + +@pytest.fixture(scope="module") +def cluster_dict(cluster_file): + with open(cluster_file) as json_file: + cluster_dict = json.load(json_file) + cluster_dict = resolve_cluster_config_placeholders(cluster_dict) + log.info("%s", cluster_dict) + return cluster_dict + + +@pytest.fixture(scope="module") +def config_dict(config_file, cluster_dict): + with open(config_file) as json_file: + config_dict_t = json.load(json_file) + config_dict = config_dict_t['rccl'] + config_dict = resolve_test_config_placeholders(config_dict, cluster_dict) + log.info("%s", config_dict) + return config_dict + + +@pytest.fixture(scope="module") +def phdl(cluster_dict): + env_vars = cluster_dict.get("env_vars") + node_list = list(cluster_dict['node_dict'].keys()) + return Pssh(log, node_list, user=cluster_dict['username'], pkey=cluster_dict['priv_key_file'], env_vars=env_vars) + + +@pytest.fixture(scope="module") +def shdl(cluster_dict): + node_list = list(cluster_dict['node_dict'].keys()) + env_vars = cluster_dict.get("env_vars") + head_node = node_list[0] + return Pssh(log, [head_node], user=cluster_dict['username'], pkey=cluster_dict['priv_key_file'], env_vars=env_vars) + + +# --------------------------------------------------------------------------- # +# Parametrization: collectives x data types x NCCL env-combos +# --------------------------------------------------------------------------- # +def pytest_generate_tests(metafunc): + config_file = metafunc.config.getoption("config_file") + if not config_file or not os.path.exists(config_file): + log.warning(f'Warning: Missing or invalid config file {config_file}') + return + + with open(config_file) as fp: + cfg = json.load(fp) + rccl = cfg.get("rccl", {}) + regression = dict(rccl.get("regression", {})) + if not regression: + log.error("No regression object found in config - required for A/B parametrization") + return + + # Paired channel handling (min/max kept paired, not Cartesian) - identical to + # the single-sided regression test. + has_min = "NCCL_MIN_NCHANNELS" in regression + has_max = "NCCL_MAX_NCHANNELS" in regression + if has_min != has_max: + raise ValueError("NCCL_MIN_NCHANNELS and NCCL_MAX_NCHANNELS must be both present or both absent") + paired_channels = None + if has_min and has_max: + min_vals = regression["NCCL_MIN_NCHANNELS"] + max_vals = regression["NCCL_MAX_NCHANNELS"] + if len(min_vals) != len(max_vals): + raise ValueError("NCCL_MIN_NCHANNELS and NCCL_MAX_NCHANNELS must have equal length") + paired_channels = list(zip(min_vals, max_vals)) + del regression["NCCL_MIN_NCHANNELS"] + del regression["NCCL_MAX_NCHANNELS"] + + env_axes = [] + for key in sorted(regression.keys()): + value = regression[key] + if isinstance(value, list) and value: + env_axes.append((key, value)) + + if env_axes and "rccl_collective" in metafunc.fixturenames: + rccl_collective_list = rccl.get("rccl_collective", ["all_reduce_perf"]) + env_fixture_names = [name for name, _ in env_axes] + env_domains = [dict(env_axes)[name] for name in env_fixture_names] + env_params, env_ids = [], [] + + channel_fixture_names = [] + if paired_channels is not None: + channel_fixture_names = ["NCCL_MIN_NCHANNELS", "NCCL_MAX_NCHANNELS"] + env_domains.append(paired_channels) + + for env_combo in itertools.product(*env_domains): + env_dict = dict(zip(env_fixture_names + channel_fixture_names, env_combo)) + if paired_channels is not None: + min_ch, max_ch = env_dict.pop("NCCL_MIN_NCHANNELS") + env_dict["NCCL_MIN_NCHANNELS"] = min_ch + env_dict["NCCL_MAX_NCHANNELS"] = max_ch + env_params.append(env_dict) + env_ids.append("|".join(f"{k}={v}" for k, v in env_dict.items())) + + metafunc.parametrize("rccl_collective", rccl_collective_list) + metafunc.parametrize("regression_params", env_params, ids=env_ids) + + # Optional data-type axis. Each data type runs as its own sweep (rccl-tests -d). + if "data_type" in metafunc.fixturenames: + data_type_list = rccl.get("data_types", ["float"]) + metafunc.parametrize("data_type", data_type_list) + + +# --------------------------------------------------------------------------- # +# Helpers +# --------------------------------------------------------------------------- # +def _side_params(base_rccl_test_params, side_cfg, data_type=None): + """Build a per-side copy of rccl_test_params with the build's tests dir / lib path.""" + params = copy.deepcopy(base_rccl_test_params) + if side_cfg.get("rccl_tests_dir"): + params["rccl_tests_dir"] = side_cfg["rccl_tests_dir"] + if side_cfg.get("ld_library_path"): + params["ld_library_path"] = side_cfg["ld_library_path"] + if data_type: + params["data_type"] = data_type + return params + + +def _run_one_side(phdl, shdl, cluster_dict, config_dict, side_cfg, collective, env_overrides, repeat_idx, + data_type=None): + """Run a single sweep for one build side and return the parsed result rows.""" + node_list = list(cluster_dict['node_dict'].keys()) + vpc_node_list = [cluster_dict['node_dict'][n]['vpc_ip'] for n in node_list] + + rccl_test_params = _side_params(config_dict['rccl_test_params'], side_cfg, data_type) + + # Per-side, per-repeat result file so concurrent/sequential runs never clash. + base_cvs = copy.deepcopy(config_dict['cvs_params']) + base_file = base_cvs.get('rccl_result_file', '/tmp/rccl_result_output.json') + stem, ext = os.path.splitext(base_file) + label = side_cfg.get("label", "side") + dt = data_type or "na" + base_cvs['rccl_result_file'] = f'{stem}_{label}_{dt}_r{repeat_idx}{ext}' + # A/B does its own comparison; disable the single-sided verification entirely. + base_cvs['verify_bus_bw'] = 'False' + base_cvs['verify_bw_dip'] = 'False' + base_cvs['verify_lat_dip'] = 'False' + + env_overrides = {k: str(v) for k, v in env_overrides.items()} + + return rccl_lib.rccl_regression( + phdl, + shdl, + collective, + config_dict.get('env_source_script', '/dev/null'), + config_dict['mpi_params'], + rccl_test_params, + base_cvs, + node_list, + vpc_node_list, + env_overrides, + ) + + +# --------------------------------------------------------------------------- # +# Tests +# --------------------------------------------------------------------------- # +def test_ab_pair(phdl, shdl, cluster_dict, config_dict, rccl_collective, regression_params, data_type): + """Run reference (A) and candidate (B) interleaved for R repeats and stash samples.""" + globals.error_list = [] + + ab_cfg = config_dict.get('ab_regression', {}) + repeats = int(ab_cfg.get('repeats', 7)) + control_mode = bool(ab_cfg.get('control_mode', False)) + + reference_cfg = ab_cfg.get('reference', {"label": "ref"}) + candidate_cfg = ab_cfg.get('candidate', {"label": "cand"}) + if control_mode: + # Both sides use the reference build to characterise noise. + candidate_cfg = copy.deepcopy(reference_cfg) + candidate_cfg['label'] = 'cand' + reference_cfg = {**reference_cfg, "label": reference_cfg.get("label", "ref")} + candidate_cfg = {**candidate_cfg, "label": candidate_cfg.get("label", "cand")} + + params_str = ' '.join(f'{k}={v}' for k, v in regression_params.items()) + group_key = f'{rccl_collective}-d={data_type}-{params_str}' + ab_runs.setdefault(group_key, {"a": [], "b": []}) + + for r in range(repeats): + # Interleave A,B per repeat so slow drift affects both sides equally. + a_rows = _run_one_side( + phdl, shdl, cluster_dict, config_dict, reference_cfg, rccl_collective, regression_params, r, data_type + ) + b_rows = _run_one_side( + phdl, shdl, cluster_dict, config_dict, candidate_cfg, rccl_collective, regression_params, r, data_type + ) + ab_runs[group_key]["a"].append(a_rows) + ab_runs[group_key]["b"].append(b_rows) + + update_test_result() + + +def test_ab_analyze(request, config_dict): + """Derive thresholds (control mode) and/or run the A/B detector; fail on confirmed regressions.""" + globals.error_list = [] + ab_cfg = config_dict.get('ab_regression', {}) + control_mode = bool(ab_cfg.get('control_mode', False)) + + # Effective detector config from the user's ab_regression block. + detector_overrides = {} + for k in ("thresholds", "tier_boundaries", "separation_gate", "separation_b_percentile", + "separation_a_percentile", "adjacency_min_run", "min_bandwidth_floor", "min_repeats", + "metric", "higher_is_better"): + if k in ab_cfg: + detector_overrides[k] = ab_cfg[k] + + out_dir = ab_cfg.get('output_dir') or os.getenv('CVS_OUTPUT_BASE_DIR') or '/tmp' + os.makedirs(out_dir, exist_ok=True) + + overall_has_regression = False + all_reports = {} + + # Control mode: derive thresholds from the combined A+B (same build) data. + if control_mode: + control_runs = [] + for g in ab_runs.values(): + control_runs.extend(g["a"]) + control_runs.extend(g["b"]) + if control_runs: + derived = regression_lib.derive_thresholds( + control_runs, + config=detector_overrides or None, + safety_factor=float(ab_cfg.get('safety_factor', 2.0)), + ) + log.info("Derived thresholds from control run: %s", derived["thresholds"]) + log.info("Measured noise: %s", derived["noise"]) + with open(os.path.join(out_dir, 'ab_derived_thresholds.json'), 'w') as fp: + json.dump(derived, fp, indent=2) + # Apply derived thresholds for the (sanity) detection below. + detector_overrides = {**detector_overrides, "thresholds": derived["thresholds"]} + + for group_key, runs in ab_runs.items(): + report = regression_lib.detect_regressions(runs["a"], runs["b"], config=detector_overrides or None) + all_reports[group_key] = report + log.info("[%s]\n%s", group_key, regression_lib.format_report(report)) + if report["summary"]["has_regression"]: + overall_has_regression = True + + with open(os.path.join(out_dir, 'ab_regression_report.json'), 'w') as fp: + json.dump({"control_mode": control_mode, "reports": all_reports}, fp, indent=2, default=str) + + if control_mode: + # In control mode a confirmed regression means the detector is NOT stable + # on this hardware (identical build flagged) - that must fail loudly. + if overall_has_regression: + fail_test("Control run (A=B) reported a regression - detector/thresholds are not stable on this hardware") + else: + if overall_has_regression: + regressions = sum(r["summary"]["regressions"] for r in all_reports.values()) + fail_test(f"A/B regression detected: {regressions} confirmed regression(s) - see ab_regression_report.json") + + update_test_result() From b0fd80bc4a64686c6f57cbb5d49c433add0403dc Mon Sep 17 00:00:00 2001 From: surya periaswamy Date: Wed, 3 Jun 2026 21:13:39 -0500 Subject: [PATCH 02/17] AIMVT-196: add test retry + stale-GPU cleanup robustness features Implement the remaining ticket items, keeping pure decision logic separate from cluster orchestration and unit-testing it (14 new tests, 61 total): - ci_robustness_lib.py: - run_with_retries(): retry transient sweep failures with linear backoff and a pre-retry hook; classify_failure() never retries data-corruption/schema failures (so real bugs aren't papered over). - build_gpu_cleanup_script()/parse_gpu_pids(): kill stale RCCL/MPI processes (self-match-safe pkill via the [x]yz trick), optionally GPU-holding PIDs from rocm-smi and stale docker/podman containers. - rccl_lib.cleanup_gpus_on_nodes(): best-effort cluster-wide cleanup wrapper. - rccl_ab_regression.py: run cleanup first (test_00) and between sweep retries; wrap each A/B sweep in run_with_retries (transparent on healthy runs). Validated on 4-node MI350X: cleanup runs per node, retry is transparent on success, control verdict unchanged (0 regressions). Co-authored-by: Cursor --- cvs/lib/ci_robustness_lib.py | 271 ++++++++++++++++++++ cvs/lib/rccl_lib.py | 56 ++++ cvs/lib/unittests/test_ci_robustness_lib.py | 128 +++++++++ cvs/tests/rccl/rccl_ab_regression.py | 91 ++++++- 4 files changed, 542 insertions(+), 4 deletions(-) create mode 100644 cvs/lib/ci_robustness_lib.py create mode 100644 cvs/lib/unittests/test_ci_robustness_lib.py diff --git a/cvs/lib/ci_robustness_lib.py b/cvs/lib/ci_robustness_lib.py new file mode 100644 index 000000000..55117f909 --- /dev/null +++ b/cvs/lib/ci_robustness_lib.py @@ -0,0 +1,271 @@ +''' +Copyright 2025 Advanced Micro Devices, Inc. +All rights reserved. This notice is intended as a precaution against inadvertent publication and does not imply publication or any waiver of confidentiality. +The year included in the foregoing notice is the year of creation of the work. +All code contained here is Property of Advanced Micro Devices, Inc. +''' + +""" +CI robustness helpers for the RCCL regression pipeline (AIMVT-196): + + 1. Retry of transient test failures (run_with_retries / classify_failure). + 2. Killing stale GPU-holding processes / containers before a run + (build_gpu_cleanup_script / parse_gpu_pids). + +The decision logic here is intentionally pure and dependency-free so it can be +unit-tested exhaustively on a login node. The thin cluster-facing wrapper that +actually executes the cleanup over SSH lives in rccl_lib.cleanup_gpus_on_nodes +and reuses the builders below. +""" + +import re +import shlex +import time + +# --------------------------------------------------------------------------- +# Retry +# --------------------------------------------------------------------------- + +# Substrings/regexes that indicate a *transient* failure worth retrying. +DEFAULT_RETRIABLE_PATTERNS = [ + r'NCCL ERROR', + r'unhandled system error', + r'unhandled (cuda|hip) error', + r'Test NCCL failure', + r'ORTE', + r'PML add procs failed', + r'MPI_Init', + r'ompi_mpi_init', + r'Connection (refused|reset|timed out)', + r'connection closed', + r'timed out|timeout', + r'Hit Exceptions', + r'no bandwidth numbers', + r'No route to host', + r'socket', + r'Software caused connection abort', + r'remote process|process exited|exited on signal', + r'no result rows', +] + +# Substrings/regexes that indicate a *real* failure that must NOT be retried +# (retrying would only hide a genuine bug). These take precedence. +DEFAULT_NON_RETRIABLE_PATTERNS = [ + r'SEVERE DATA CORRUPTION', + r"'#wrong'", + r'wrong=', + r'schema validation failed', +] + + +def classify_failure(message, retriable_patterns=None, non_retriable_patterns=None): + """ + Decide whether a failure message describes a transient (retriable) error. + + Non-retriable patterns (e.g. data corruption) are checked first and win: + we never want to paper over a correctness failure by retrying. + + Args: + message: failure text (str or anything str()-able, e.g. an exception). + retriable_patterns / non_retriable_patterns: optional override lists. + + Returns: + bool: True if the failure looks transient and should be retried. + """ + text = str(message) + non_retriable = non_retriable_patterns if non_retriable_patterns is not None else DEFAULT_NON_RETRIABLE_PATTERNS + retriable = retriable_patterns if retriable_patterns is not None else DEFAULT_RETRIABLE_PATTERNS + + for pat in non_retriable: + if re.search(pat, text, re.IGNORECASE): + return False + for pat in retriable: + if re.search(pat, text, re.IGNORECASE): + return True + # Unknown failures: default to retriable. A transient infra blip is the common + # case in multi-node CI; genuine bugs are caught by the non-retriable list and + # by the fact that a real regression reproduces on every retry anyway. + return True + + +def run_with_retries( + attempt_fn, + max_retries=2, + is_retriable=None, + on_before_retry=None, + backoff_sec=0, + sleep_fn=time.sleep, + log=None, + label="task", +): + """ + Call ``attempt_fn()`` up to ``max_retries + 1`` times. + + ``attempt_fn`` must raise an exception on failure and return a value on + success. After a failing attempt, if more attempts remain and the failure is + retriable, ``on_before_retry(next_attempt_index)`` is invoked (e.g. to clean + up stale GPU state), then we sleep ``backoff_sec * attempt`` (linear backoff) + and try again. + + Args: + attempt_fn: zero-arg callable; raises on failure. + max_retries: number of *extra* attempts after the first (so total = +1). + is_retriable: callable(exc) -> bool. Defaults to classify_failure(str(exc)). + on_before_retry: optional callable(next_attempt_index:int) run between attempts. + backoff_sec: base backoff seconds (linear: backoff_sec * attempt_number). + sleep_fn: injectable sleep (for tests). + log: optional logger. + label: short description for log lines. + + Returns: + Whatever attempt_fn() returns on the first successful attempt. + + Raises: + The last exception if all attempts fail or a failure is non-retriable. + """ + if is_retriable is None: + is_retriable = lambda exc: classify_failure(exc) + total_attempts = max(1, max_retries + 1) + last_exc = None + for attempt in range(1, total_attempts + 1): + try: + return attempt_fn() + except Exception as exc: # noqa: BLE001 - we deliberately catch broadly to retry + last_exc = exc + retriable = bool(is_retriable(exc)) + more_attempts = attempt < total_attempts + if log is not None: + log.warning( + "%s attempt %d/%d failed: %r (retriable=%s)", + label, attempt, total_attempts, exc, retriable, + ) + if not (more_attempts and retriable): + raise + if on_before_retry is not None: + try: + on_before_retry(attempt + 1) + except Exception as cleanup_exc: # noqa: BLE001 + if log is not None: + log.warning("%s on_before_retry hook failed (ignored): %r", label, cleanup_exc) + if backoff_sec: + sleep_fn(backoff_sec * attempt) + # Should be unreachable, but re-raise the last error defensively. + raise last_exc + + +# --------------------------------------------------------------------------- +# GPU / container cleanup +# --------------------------------------------------------------------------- + +# Default process-name patterns for stale RCCL/MPI leftovers from prior or +# killed jobs. On an exclusive compute node any match is stale by definition. +DEFAULT_GPU_PROCESS_PATTERNS = [ + 'all_reduce_perf', + 'all_reduce_bias_perf', + 'all_gather_perf', + 'reduce_scatter_perf', + 'broadcast_perf', + 'alltoall_perf', + 'alltoallv_perf', + 'sendrecv_perf', + 'scatter_perf', + 'gather_perf', + 'reduce_perf', + 'hypercube_perf', + 'mpirun', + 'orted', + 'prted', +] + + +def parse_gpu_pids(rocm_smi_showpids_output): + """ + Extract PIDs from the output of ``rocm-smi --showpids``. + + The table rows begin with a numeric PID; header/border lines do not. We pull + the leading integer from each line and drop obvious non-PIDs (0/1). + + Args: + rocm_smi_showpids_output: str output of `rocm-smi --showpids`. + + Returns: + sorted list[int] of unique candidate PIDs. + """ + pids = set() + for line in (rocm_smi_showpids_output or "").splitlines(): + m = re.match(r'^\s*(\d+)\b', line) + if not m: + continue + pid = int(m.group(1)) + if pid > 1: + pids.add(pid) + return sorted(pids) + + +def _self_safe_pattern(pattern): + """ + Rewrite a pkill -f pattern so it cannot match the cleanup command's own + command line. Wrapping the first alphanumeric character in a regex class + (e.g. 'orted' -> '[o]rted') matches the target process but not the literal + pattern text present in our own argv. Classic pgrep/pkill self-match guard. + """ + for i, ch in enumerate(pattern): + if ch.isalnum(): + return pattern[:i] + '[' + ch + ']' + pattern[i + 1:] + return pattern + + +def build_gpu_cleanup_script( + process_patterns=None, + kill_gpu_pids=True, + kill_containers=False, + use_sudo=False, +): + """ + Build a best-effort bash script that clears stale GPU state before a run. + + Steps (all guarded with ``|| true`` so cleanup never fails the job): + 1. pkill -9 -f each known RCCL/MPI process pattern (self-match-safe). + 2. Optionally kill every PID reported by ``rocm-smi --showpids``. + 3. Optionally stop stale GPU containers (docker/podman). + + Args: + process_patterns: list of process-name substrings (defaults to RCCL/MPI set). + kill_gpu_pids: also kill PIDs reported by rocm-smi. + kill_containers: also kill running docker/podman containers. + use_sudo: prefix kill/pkill/container commands with sudo. + + Returns: + str: a bash script. + """ + patterns = process_patterns if process_patterns is not None else DEFAULT_GPU_PROCESS_PATTERNS + sudo = 'sudo ' if use_sudo else '' + lines = [ + '#!/usr/bin/env bash', + '# Auto-generated stale-GPU cleanup (AIMVT-196). Best-effort; never fails.', + 'set +e', + 'echo "[gpu-cleanup] host=$(hostname) start"', + ] + + for pat in patterns: + safe = shlex.quote(_self_safe_pattern(pat)) + lines.append(f'{sudo}pkill -9 -f -- {safe} 2>/dev/null || true') + + if kill_gpu_pids: + lines.append('if command -v rocm-smi >/dev/null 2>&1; then') + lines.append(" for p in $(rocm-smi --showpids 2>/dev/null | awk '/^[0-9]+/{print $1}'); do") + lines.append(f' {sudo}kill -9 "$p" 2>/dev/null || true') + lines.append(' done') + lines.append('fi') + + if kill_containers: + lines.append('if command -v docker >/dev/null 2>&1; then') + lines.append(f' {sudo}docker ps -q 2>/dev/null | xargs -r {sudo}docker kill >/dev/null 2>&1 || true') + lines.append('fi') + lines.append('if command -v podman >/dev/null 2>&1; then') + lines.append(f' {sudo}podman ps -q 2>/dev/null | xargs -r {sudo}podman kill >/dev/null 2>&1 || true') + lines.append('fi') + + lines.append('echo "[gpu-cleanup] host=$(hostname) done"') + lines.append('true') + return '\n'.join(lines) diff --git a/cvs/lib/rccl_lib.py b/cvs/lib/rccl_lib.py index b314e105e..006f5b123 100644 --- a/cvs/lib/rccl_lib.py +++ b/cvs/lib/rccl_lib.py @@ -18,6 +18,7 @@ from pydantic import ValidationError from cvs.lib import globals +from cvs.lib import ci_robustness_lib from cvs.schema.rccl import RcclTests, RcclTestsAggregated, RcclTestsMultinodeRaw from cvs.lib.utils_lib import * from cvs.lib.verify_lib import * @@ -25,6 +26,61 @@ log = globals.log +def cleanup_gpus_on_nodes( + phdl, + process_patterns=None, + kill_gpu_pids=True, + kill_containers=False, + use_sudo=False, + timeout=120, +): + """ + Kill stale RCCL/MPI processes (and optionally GPU-holding PIDs / containers) + on every node reachable through ``phdl`` before launching a test. + + Intended to be run on exclusively-allocated compute nodes, where any leftover + rccl-tests/mpirun/orted process is stale from a prior or cancelled job and can + otherwise hold GPUs / NIC state and make the next run flaky. + + Best-effort: failures are logged but never raise (cleanup must not break a run). + + Returns: + dict[node] -> command output (for logging/auditing). + """ + # Log what is currently holding the GPUs (audit trail), best-effort. + try: + discover = phdl.exec('rocm-smi --showpids 2>/dev/null || true') + for node, out in (discover or {}).items(): + pids = ci_robustness_lib.parse_gpu_pids(out) + if pids: + log.warning('Pre-clean GPU PIDs on %s: %s', node, pids) + else: + log.info('No GPU PIDs reported on %s before cleanup', node) + except Exception as e: + log.warning('GPU PID discovery failed (ignored): %r', e) + + script = ci_robustness_lib.build_gpu_cleanup_script( + process_patterns=process_patterns, + kill_gpu_pids=kill_gpu_pids, + kill_containers=kill_containers, + use_sudo=use_sudo, + ) + # Wrap in bash -c so the multi-line script runs as one command per node. + cmd = "bash -c " + _shell_single_quote(script) + try: + out_dict = phdl.exec(cmd, timeout=timeout) + log.info('Stale-GPU cleanup completed on %d node(s)', len(out_dict or {})) + return out_dict + except Exception as e: + log.warning('Stale-GPU cleanup exec failed (ignored): %r', e) + return {} + + +def _shell_single_quote(s): + """Single-quote a string for safe embedding in a bash -c argument.""" + return "'" + s.replace("'", "'\"'\"'") + "'" + + rccl_err_dict = { 'orte': 'ORTE does not know how to route|ORTE was unable to reliably start', 'nccl': 'NCCL ERROR|Test failure', diff --git a/cvs/lib/unittests/test_ci_robustness_lib.py b/cvs/lib/unittests/test_ci_robustness_lib.py new file mode 100644 index 000000000..d846c88d9 --- /dev/null +++ b/cvs/lib/unittests/test_ci_robustness_lib.py @@ -0,0 +1,128 @@ +# cvs/lib/unittests/test_ci_robustness_lib.py +"""Unit tests for retry + GPU-cleanup helpers (cvs.lib.ci_robustness_lib).""" + +import unittest + +import cvs.lib.ci_robustness_lib as rb + + +class TestClassifyFailure(unittest.TestCase): + def test_retriable_patterns(self): + self.assertTrue(rb.classify_failure("NCCL ERROR: remote process exited")) + self.assertTrue(rb.classify_failure("MPI_Init: PML add procs failed")) + self.assertTrue(rb.classify_failure("Connection timed out")) + self.assertTrue(rb.classify_failure("no result rows returned")) + + def test_non_retriable_wins(self): + # Even though it also contains a retriable-ish word, corruption must not retry. + self.assertFalse(rb.classify_failure("SEVERE DATA CORRUPTION: NCCL ERROR")) + self.assertFalse(rb.classify_failure("RCCL Test float schema validation failed: ...")) + self.assertFalse(rb.classify_failure("rccl-tests reported '#wrong' = 5")) + + def test_unknown_defaults_retriable(self): + self.assertTrue(rb.classify_failure("some unfamiliar transient blip")) + + def test_accepts_exception_objects(self): + self.assertFalse(rb.classify_failure(RuntimeError("SEVERE DATA CORRUPTION"))) + self.assertTrue(rb.classify_failure(RuntimeError("socket closed"))) + + +class TestRunWithRetries(unittest.TestCase): + def test_success_first_attempt(self): + calls = [] + out = rb.run_with_retries(lambda: (calls.append(1), "ok")[1], max_retries=3) + self.assertEqual(out, "ok") + self.assertEqual(len(calls), 1) + + def test_success_after_retries(self): + state = {"n": 0} + + def flaky(): + state["n"] += 1 + if state["n"] < 3: + raise RuntimeError("NCCL ERROR transient") + return "recovered" + + sleeps = [] + retries = [] + out = rb.run_with_retries( + flaky, max_retries=3, backoff_sec=5, + sleep_fn=lambda s: sleeps.append(s), + on_before_retry=lambda n: retries.append(n), + ) + self.assertEqual(out, "recovered") + self.assertEqual(state["n"], 3) # failed twice, succeeded on 3rd + self.assertEqual(retries, [2, 3]) # hook ran before attempts 2 and 3 + self.assertEqual(sleeps, [5, 10]) # linear backoff 5*1, 5*2 + + def test_exhausts_and_raises(self): + state = {"n": 0} + + def always_fail(): + state["n"] += 1 + raise RuntimeError("timeout") + + with self.assertRaises(RuntimeError): + rb.run_with_retries(always_fail, max_retries=2, sleep_fn=lambda s: None) + self.assertEqual(state["n"], 3) # 1 + 2 retries + + def test_non_retriable_raises_immediately(self): + state = {"n": 0} + + def corrupt(): + state["n"] += 1 + raise RuntimeError("SEVERE DATA CORRUPTION") + + with self.assertRaises(RuntimeError): + rb.run_with_retries(corrupt, max_retries=5, sleep_fn=lambda s: None) + self.assertEqual(state["n"], 1) # never retried + + +class TestParseGpuPids(unittest.TestCase): + def test_parse_typical_output(self): + out = """ +========================= ROCm System Management Interface ========================= +================================== KFD Processes =================================== +PID PROCESS NAME GPU(s) VRAM USED +12345 all_reduce_perf 8 1234567 +12346 all_reduce_perf 8 1234567 +0 systemd 0 0 +==================================================================================== +""" + pids = rb.parse_gpu_pids(out) + self.assertEqual(pids, [12345, 12346]) # header text + pid 0 excluded + + def test_empty(self): + self.assertEqual(rb.parse_gpu_pids(""), []) + self.assertEqual(rb.parse_gpu_pids(None), []) + + +class TestBuildCleanupScript(unittest.TestCase): + def test_self_safe_patterns_and_pkill(self): + script = rb.build_gpu_cleanup_script(process_patterns=["all_reduce_perf", "orted"]) + # Bracketized (self-match-safe) forms present... + self.assertIn("[a]ll_reduce_perf", script) + self.assertIn("[o]rted", script) + # ...and the bare forms are NOT used as a standalone pkill target. + self.assertNotIn("pkill -9 -f -- all_reduce_perf", script) + self.assertIn("pkill -9 -f", script) + + def test_gpu_pids_block_toggle(self): + with_pids = rb.build_gpu_cleanup_script(kill_gpu_pids=True) + without = rb.build_gpu_cleanup_script(kill_gpu_pids=False) + self.assertIn("rocm-smi --showpids", with_pids) + self.assertNotIn("rocm-smi --showpids", without) + + def test_container_block_toggle(self): + with_c = rb.build_gpu_cleanup_script(kill_containers=True) + without = rb.build_gpu_cleanup_script(kill_containers=False) + self.assertIn("docker kill", with_c) + self.assertNotIn("docker kill", without) + + def test_sudo_prefix(self): + s = rb.build_gpu_cleanup_script(process_patterns=["orted"], use_sudo=True) + self.assertIn("sudo pkill", s) + + +if __name__ == "__main__": + unittest.main() diff --git a/cvs/tests/rccl/rccl_ab_regression.py b/cvs/tests/rccl/rccl_ab_regression.py index de414b6a0..49339e1e4 100644 --- a/cvs/tests/rccl/rccl_ab_regression.py +++ b/cvs/tests/rccl/rccl_ab_regression.py @@ -35,6 +35,7 @@ from cvs.lib import rccl_lib from cvs.lib import regression_lib +from cvs.lib import ci_robustness_lib from cvs.lib.parallel_ssh_lib import * from cvs.lib.utils_lib import * from cvs.lib.verify_lib import * @@ -43,6 +44,32 @@ log = globals.log +def _gpu_cleanup_cfg(config_dict): + """Return the gpu_cleanup config block with sane defaults.""" + cfg = dict(config_dict.get('gpu_cleanup', {})) + cfg.setdefault('enabled', True) + cfg.setdefault('kill_gpu_pids', True) + cfg.setdefault('kill_containers', False) + cfg.setdefault('use_sudo', False) + cfg.setdefault('process_patterns', None) # None -> library defaults + return cfg + + +def _do_gpu_cleanup(phdl, config_dict, reason=""): + """Run stale-GPU cleanup across all nodes if enabled in config.""" + cfg = _gpu_cleanup_cfg(config_dict) + if not cfg.get('enabled', True): + return + log.info("Running stale-GPU cleanup%s", f" ({reason})" if reason else "") + rccl_lib.cleanup_gpus_on_nodes( + phdl, + process_patterns=cfg.get('process_patterns'), + kill_gpu_pids=cfg.get('kill_gpu_pids', True), + kill_containers=cfg.get('kill_containers', False), + use_sudo=cfg.get('use_sudo', False), + ) + + # Accumulates per-(collective, dtype, env-combo) A and B run samples across the # parametrized test invocations, consumed by the final analysis test. # ab_runs[group_key] = {"a": [run, run, ...], "b": [run, run, ...]} @@ -179,9 +206,15 @@ def _side_params(base_rccl_test_params, side_cfg, data_type=None): return params -def _run_one_side(phdl, shdl, cluster_dict, config_dict, side_cfg, collective, env_overrides, repeat_idx, - data_type=None): - """Run a single sweep for one build side and return the parsed result rows.""" +def _run_one_side_once(phdl, shdl, cluster_dict, config_dict, side_cfg, collective, env_overrides, repeat_idx, + data_type=None): + """ + Run a single sweep for one build side and return the parsed result rows. + + Raises on failure so the retry wrapper can act: a sweep "fails" if + rccl_regression raises, if it recorded errors via fail_test (error_list), or + if it produced no result rows. + """ node_list = list(cluster_dict['node_dict'].keys()) vpc_node_list = [cluster_dict['node_dict'][n]['vpc_ip'] for n in node_list] @@ -201,7 +234,10 @@ def _run_one_side(phdl, shdl, cluster_dict, config_dict, side_cfg, collective, e env_overrides = {k: str(v) for k, v in env_overrides.items()} - return rccl_lib.rccl_regression( + # Reset the global failure accumulator so we can detect this sweep's own + # failures (rccl_regression records via fail_test rather than always raising). + globals.error_list = [] + rows = rccl_lib.rccl_regression( phdl, shdl, collective, @@ -213,11 +249,58 @@ def _run_one_side(phdl, shdl, cluster_dict, config_dict, side_cfg, collective, e vpc_node_list, env_overrides, ) + if globals.error_list: + errs = list(globals.error_list) + globals.error_list = [] + raise RuntimeError(f"sweep reported failures: {errs}") + if not rows: + raise RuntimeError("sweep returned no result rows") + return rows + + +def _run_one_side(phdl, shdl, cluster_dict, config_dict, side_cfg, collective, env_overrides, repeat_idx, + data_type=None): + """Run one sweep with retry on transient failures; clean stale GPU state between attempts.""" + retry_cfg = config_dict.get('retry', {}) + max_retries = int(retry_cfg.get('max_retries', 2)) + backoff_sec = float(retry_cfg.get('backoff_sec', 15)) + + def attempt(): + return _run_one_side_once( + phdl, shdl, cluster_dict, config_dict, side_cfg, collective, env_overrides, repeat_idx, data_type + ) + + def on_before_retry(next_attempt): + # A flaky run can leave orphaned ranks/orted holding GPUs; clear them first. + _do_gpu_cleanup(phdl, config_dict, reason=f"before retry {next_attempt} of {side_cfg.get('label')}") + + result = ci_robustness_lib.run_with_retries( + attempt, + max_retries=max_retries, + on_before_retry=on_before_retry, + backoff_sec=backoff_sec, + log=log, + label=f"{collective}/{side_cfg.get('label')}/d={data_type}/r{repeat_idx}", + ) + # Ensure no stale errors leak into the test's final pass/fail check. + globals.error_list = [] + return result # --------------------------------------------------------------------------- # # Tests # --------------------------------------------------------------------------- # +def test_00_cleanup_stale_gpu_state(phdl, config_dict): + """ + Kill stale RCCL/MPI processes (and optionally GPU PIDs / containers) left on + the allocated nodes by prior or cancelled jobs, before any benchmark runs. + Runs first by virtue of its position in this module. Best-effort: never fails. + """ + globals.error_list = [] + _do_gpu_cleanup(phdl, config_dict, reason="pre-run") + update_test_result() + + def test_ab_pair(phdl, shdl, cluster_dict, config_dict, rccl_collective, regression_params, data_type): """Run reference (A) and candidate (B) interleaved for R repeats and stash samples.""" globals.error_list = [] From 02d0eb36591efb81292ca941cf5345d2c155a0c6 Mon Sep 17 00:00:00 2001 From: surya periaswamy Date: Thu, 4 Jun 2026 17:52:34 -0500 Subject: [PATCH 03/17] AIMVT-196: clean per-run log with MPI launch command + rccl-tests output Add a dedicated, low-noise run record separate from the verbose parallel-ssh logging. For each rccl-tests run we append a section to cvs_params.rccl_command_log containing the MPI launch command normalized to a single copy/paste-able line (for reproducibility) followed by the raw rccl-tests output (perf table; plus NCCL INFO when NCCL_DEBUG=INFO). - rccl_lib.format_run_command_log_entry() (pure, unit-tested) + _maybe_write_run_command_log() - rccl_regression appends the record on a successful run - rccl_ab_regression auto-defaults the path to /rccl_runs.log Co-authored-by: Cursor --- cvs/lib/rccl_lib.py | 69 ++++++++++++++++++++++++++++ cvs/lib/unittests/test_rccl_lib.py | 19 ++++++++ cvs/tests/rccl/rccl_ab_regression.py | 8 ++++ 3 files changed, 96 insertions(+) diff --git a/cvs/lib/rccl_lib.py b/cvs/lib/rccl_lib.py index 006f5b123..3b3e12f86 100644 --- a/cvs/lib/rccl_lib.py +++ b/cvs/lib/rccl_lib.py @@ -81,6 +81,72 @@ def _shell_single_quote(s): return "'" + s.replace("'", "'\"'\"'") + "'" +def format_run_command_log_entry(label, command, output): + """ + Build one clean, reproducible log section for a single rccl-tests run. + + The MPI launch command is normalized to a single copy/paste-able line and + placed at the top, followed by the raw rccl-tests output (the perf table, and + NCCL INFO lines when NCCL_DEBUG=INFO). This is the high-signal content people + actually want, separated from the verbose per-line parallel-ssh logging. + + Args: + label: short description of the run (collective / env / result file). + command: the mpirun launch command (may contain shell line continuations). + output: the raw rccl-tests stdout/stderr captured from the head node. + + Returns: + str: a formatted section (begins with a blank line + separator). + """ + from datetime import datetime + + # Collapse shell line-continuations and runs of whitespace into a single line + # so the command can be copied and re-run as-is. + one_line = re.sub(r'\\\s*\n', ' ', command or '') + one_line = re.sub(r'\s+', ' ', one_line).strip() + + sep = '=' * 100 + parts = [ + '', + sep, + f'# RUN : {label}', + f'# TIME: {datetime.now().strftime("%Y-%m-%d %H:%M:%S")}', + sep, + '# ---- MPI launch command (copy/paste to reproduce) ----', + one_line, + '', + '# ---- rccl-tests output (perf table; NCCL INFO when NCCL_DEBUG=INFO) ----', + (output or '').rstrip(), + '', + ] + return '\n'.join(parts) + + +def _maybe_write_run_command_log(cvs_params, test_name, env_overrides, rccl_result_file, command, output): + """ + Append a clean run record (launch command + rccl-tests output) to the file at + ``cvs_params['rccl_command_log']`` if configured. Best-effort: never raises. + + This is intentionally separate from (and in addition to) the verbose parallel-ssh + logging, which is preserved unchanged. + """ + log_path = cvs_params.get('rccl_command_log') + if not log_path: + return + params_summary = ' '.join(f'{k}={v}' for k, v in (env_overrides or {}).items()) + label = test_name + if params_summary: + label += f' [{params_summary}]' + label += f' -> {os.path.basename(rccl_result_file)}' + try: + entry = format_run_command_log_entry(label, command, output) + with open(log_path, 'a', encoding='utf-8') as fp: + fp.write(entry) + log.info('Appended reproducible run record to %s', log_path) + except Exception as e: + log.warning('Failed to write rccl command log %s: %r', log_path, e) + + rccl_err_dict = { 'orte': 'ORTE does not know how to route|ORTE was unable to reliably start', 'nccl': 'NCCL ERROR|Test failure', @@ -814,6 +880,9 @@ def rccl_regression( out_dict = shdl.exec(cmd, timeout=cvs_exec_timeout) output = out_dict[head_node] scan_rccl_logs(output) + # Write a clean, reproducible record (launch command + rccl-tests output) + # to a dedicated log file, separate from the verbose parallel-ssh logging. + _maybe_write_run_command_log(cvs_params, test_name, env_overrides, rccl_result_file, cmd, output) except Exception as e: log.error(f'Hit Exceptions with rccl cmd {cmd} - exception {repr(e)}') fail_test(f'Hit Exceptions with rccl cmd {cmd} - exception {repr(e)}') diff --git a/cvs/lib/unittests/test_rccl_lib.py b/cvs/lib/unittests/test_rccl_lib.py index d06fcbf8e..d40af3953 100644 --- a/cvs/lib/unittests/test_rccl_lib.py +++ b/cvs/lib/unittests/test_rccl_lib.py @@ -368,6 +368,25 @@ def test_check_lat_dip_multi_dtype_no_false_positive(self, mock_fail_test): rccl_lib.check_lat_dip(test_name, output, ref) mock_fail_test.assert_not_called() + def test_format_run_command_log_entry(self): + """Clean run record: command collapsed to one line; output + headers present.""" + command = ( + "/opt/ompi/bin/mpirun \\\n" + " --allow-run-as-root \\\n" + " -np 32 \\\n" + " all_reduce_perf -b 1K -e 4G" + ) + output = "# size busbw\n 1024 0.02\n# Avg bus bandwidth : 100.0\n" + entry = rccl_lib.format_run_command_log_entry("all_reduce_perf [NCCL_ALGO=Ring] -> ref_float_r0.json", + command, output) + # Command is collapsed to a single copy/paste-able line (no backslashes/newlines). + self.assertIn("mpirun --allow-run-as-root -np 32 all_reduce_perf -b 1K -e 4G", entry) + self.assertNotIn("\\\n", entry) + # Labels + raw output preserved. + self.assertIn("# RUN : all_reduce_perf [NCCL_ALGO=Ring] -> ref_float_r0.json", entry) + self.assertIn("MPI launch command", entry) + self.assertIn("# Avg bus bandwidth : 100.0", entry) + @patch('cvs.lib.rccl_lib.fail_test') def test_check_bus_bw_multi_dtype_each_compared(self, mock_fail_test): """Both data types are compared against the size-keyed reference.""" diff --git a/cvs/tests/rccl/rccl_ab_regression.py b/cvs/tests/rccl/rccl_ab_regression.py index 49339e1e4..dc246f520 100644 --- a/cvs/tests/rccl/rccl_ab_regression.py +++ b/cvs/tests/rccl/rccl_ab_regression.py @@ -232,6 +232,14 @@ def _run_one_side_once(phdl, shdl, cluster_dict, config_dict, side_cfg, collecti base_cvs['verify_bw_dip'] = 'False' base_cvs['verify_lat_dip'] = 'False' + # Clean, reproducible per-run log (MPI launch command + rccl-tests output), + # separate from the verbose parallel-ssh logging. Defaults to a single + # appended file under the A/B output dir; honour an explicit override. + if not base_cvs.get('rccl_command_log'): + ab_cfg = config_dict.get('ab_regression', {}) + out_dir = ab_cfg.get('output_dir') or os.getenv('CVS_OUTPUT_BASE_DIR') or '/tmp' + base_cvs['rccl_command_log'] = os.path.join(out_dir, 'rccl_runs.log') + env_overrides = {k: str(v) for k, v in env_overrides.items()} # Reset the global failure accumulator so we can detect this sweep's own From 77a410db180ce12a3d6010eb03f129664dc314dd Mon Sep 17 00:00:00 2001 From: surya periaswamy Date: Fri, 17 Jul 2026 19:28:03 +0000 Subject: [PATCH 04/17] AIMVT-196: add hang protection, skip_keys, and calibrated-threshold auto-load - Wrap remote mpirun in coreutils timeout so a wedged collective (e.g. an alltoall deadlock) is killed on the node instead of leaking ranks/GPUs indefinitely (rccl_regression, rccl_perf in rccl_lib.py). - Tolerate legacy rccl-tests runs that append duplicate JSON result blobs by parsing only the first valid value. - Make the NCCL-knob regression matrix optional in rccl_ab_regression.py so a plain perf-regression run doesn't require a knob sweep. - Add ab_regression.skip_keys to exclude known-broken upstream (collective, dtype) combos from the gate instead of hard-failing. - Auto-load hardware-calibrated thresholds (ab_derived_thresholds.json) in detect mode, with use_derived_thresholds: false as an escape hatch. - Add unit tests for the new parametrization behavior. --- RCCL_REGRESSION_DETECTOR_UPDATE.md | 362 +++++++++++++++++++++++ cvs/lib/rccl_lib.py | 54 +++- cvs/lib/unittests/test_ab_parametrize.py | 168 +++++++++++ cvs/tests/rccl/rccl_ab_regression.py | 101 +++++-- uv.lock | 3 + 5 files changed, 664 insertions(+), 24 deletions(-) create mode 100644 RCCL_REGRESSION_DETECTOR_UPDATE.md create mode 100644 cvs/lib/unittests/test_ab_parametrize.py create mode 100644 uv.lock diff --git a/RCCL_REGRESSION_DETECTOR_UPDATE.md b/RCCL_REGRESSION_DETECTOR_UPDATE.md new file mode 100644 index 000000000..335353af9 --- /dev/null +++ b/RCCL_REGRESSION_DETECTOR_UPDATE.md @@ -0,0 +1,362 @@ +# CVS RCCL Regression Strategy (AIMVT-196) + +This document describes how the CVS RCCL performance-regression pipeline detects +real RCCL performance regressions in CI **without** false positives, including the +design rationale, the algorithm, configuration, how to run it, and the evidence +that it is trustworthy. + +- **Code (cvs)** — branch `aimvt-196-rccl-regression-robustness` (origin: `ROCm/cvs`) +- **Orchestration (cvs-sbatch)** — branch `aimvt-196-rccl-regression-robustness` (origin: `speriaswamy-amd/cvs-sbatch`) +- **Companion docs**: `RCCL_REGRESSION_FINDINGS.md` (a concrete candidate regression + bisection handoff) + +--- + +## 1. Goal & guiding principles + +Run in CI as a gate on RCCL changes and answer one question reliably: +**"Did this RCCL build get slower than a known-good reference?"** across message +sizes **1 KiB → 4 GiB**. + +Priorities, in order: + +1. **No false positives.** A flaky CI gate is worse than no gate — it erodes trust + and gets ignored/disabled. Stability is the #1 requirement. +2. **Trustworthy detection of real regressions**, especially at large messages. +3. It is **acceptable to miss small regressions** (~1–2%), particularly for small, + latency-bound messages with high run-to-run variance. + +Everything below follows from these priorities. + +--- + +## 2. Why not a static baseline? + +The previous approach compared measured bus bandwidth against **hand-maintained +expected numbers** (e.g. `330`, `350` GB/s) in config. Problems: + +- CVS has **no way to compute a baseline**, so the numbers were guesses and went stale. +- Small/mid messages are **latency-bound** and noisy; a fixed threshold either + fires on noise (false positives) or is so loose it hides real regressions. +- The comparison code also had a **group-by bug** (see §6) that silently dropped + half the data. + +**Decision: replace static baselines with paired A/B testing.** + +--- + +## 3. Core idea — paired A/B testing + +Run the **candidate** build (B) and a **reference** build (A) **back-to-back, +interleaved, on the same nodes within the same SLURM allocation**, repeated N times: + +``` +repeat 1: A B +repeat 2: A B +... +repeat N: A B +``` + +Both builds are identical except for `librccl.so` (same HIP, same MPI, same GPUs, +same fabric — selected automatically via each binary's rpath). Because A and B run +in the same time window on the same hardware, environmental noise (thermals, +neighbor jobs, NIC/fabric state, slow drift) is **common-mode and cancels in the +A−B comparison**. We never ask "is this absolute number good?" (unanswerable for +small messages); we ask "is B worse than A, side-by-side, right now?" + +This is the key to small-message stability: the *absolute* small-message bandwidth +is unstable, but the *paired difference* is not. + +--- + +## 4. The detection algorithm (triple gate) + +Implemented in `cvs/cvs/lib/regression_lib.py` (pure, dependency-free, unit-tested). + +For every fully-qualified key **`(collective, size, type, inPlace)`**, we collect a +sample of bus-bandwidth measurements for A and for B (one per repeat). A key is +flagged as a regression **only if all three independent gates agree** — the +conjunction is what makes false positives extremely unlikely: + +### Gate 1 — size-tiered relative threshold +`median(B)` must be lower than `median(A)` by more than the tier's threshold: + +| tier | size range | why | +|-------|-----------------|----------------------------------| +| small | ≤ 1 MiB | latency-bound, noisiest → loosest | +| mid | 1 MiB – 64 MiB | transitional | +| large | > 64 MiB | bandwidth-bound, stable → tightest | + +Thresholds are **derived from measured noise** (see §5), not guessed. + +### Gate 2 — non-parametric separation +Require **`p75(B) < p25(A)`** — B's upper quartile below A's lower quartile, i.e. +the two distributions barely overlap. This is a distribution-free significance test +that is robust to a single straggler run and is the specific antidote to wide, +noisy small-message distributions (which overlap and therefore won't pass). + +### Gate 3 — adjacency confirmation +A candidate size is confirmed only if it belongs to a run of **≥ `adjacency_min_run` +(default 2) consecutive candidate sizes** within the same `(collective, type, +inPlace)` group. Real regressions occupy a contiguous band of sizes; isolated noise +spikes do not. + +### Safety rails +- **Median** (not mean) over repeats → robust to outlier runs. +- **`min_bandwidth_floor` (0.5 GB/s)**: the smallest sizes (~1K–64K) where busBw is + near zero and relative noise explodes are marked **`inconclusive`** and excluded + from pass/fail — we refuse to judge the region where no judgment is safe. +- **`min_repeats`**: too few samples → `inconclusive`, never a regression. +- Direction-aware: only flags **B worse than A**, never improvements. + +### Output +Per-key verdicts (`pass` / `regression` / `inconclusive`) with A/B medians, drop%, +the threshold used, and the reasons each gate passed/failed. Aggregated to a single +job verdict; any confirmed regression → the test fails (non-zero exit) → CI fails. + +--- + +## 5. Threshold calibration (control run) + +Thresholds are **measured on the actual hardware**, not picked by hand. + +1. Run in **control mode** (`control_mode: true`): the *reference* build is used as + **both** A and B. +2. Since A and B are the same build, any spread is pure run-to-run noise. We compute + the per-tier coefficient of variation and set: + + ``` + threshold[tier] = safety_factor (default 2.0) × p95(CV[tier]) # floored per tier + ``` + +3. The control run **must report 0 regressions** (A vs A). If it doesn't, the + detector/thresholds are not trustworthy on this hardware and the job fails loudly. + +Re-run control calibration whenever the **hardware, RCCL build, or cluster config** +changes. Calibrated values are written to `ab_derived_thresholds.json`. + +> Measured on 4-node MI350X (full matrix): `p95 CV` ≈ small 10% / mid 7% / large 3.7% +> → adopted thresholds **small 20% / mid 15% / large 7.5%**. + +--- + +## 6. Correctness: group-by keys + +The original comparison/report code bucketed results by **message size alone**, +silently collapsing the `(data type, inPlace)` dimensions — the last row written for +a size overwrote the others. This both **hid real regressions** (overwritten rows +vanished) and **manufactured fake ones** (a data-type boundary looked like a giant +bandwidth dip). + +Fix (in `rccl_lib.py`): +- `group_rccl_results()` — canonical grouping by `(type, inPlace)` + sort by size. +- `convert_to_graph_dict()` — expands each `(type, inPlace)` into its own series; no overwrites. +- `check_bw_dip` / `check_lat_dip` / `check_bus_bw` — group + sort before comparing. + +Comparing **like-for-like on the full key** is a prerequisite for any verdict to be +meaningful, and is the foundation the A/B detector builds on. + +--- + +## 7. Robustness features + +### Retry transient failures (`ci_robustness_lib.run_with_retries`) +- A sweep that fails transiently (NCCL/MPI bootstrap, network, timeout) is retried + up to `retry.max_retries` with linear backoff. +- **Data-corruption / schema-validation failures are never retried** + (`classify_failure`) — retrying would only hide a genuine bug. +- Retries replace a failed run (they don't add samples), so statistics stay clean. +- Transparent on healthy runs (no behavior change when nothing fails). + +### Kill stale GPU state before launch (`ci_robustness_lib.build_gpu_cleanup_script`) +- Runs **first** (test `test_00_cleanup_stale_gpu_state`) and **between retries**. +- Kills leftover RCCL/MPI processes (`pkill -f`, self-match-safe via the `[x]yz` + trick), optionally GPU-holding PIDs (`rocm-smi --showpids`) and stale + docker/podman containers. Best-effort — never fails the job. +- On exclusively-allocated nodes, any leftover process is stale by definition. + +--- + +## 8. Architecture / code layout + +Pure decision logic is separated from cluster orchestration so it can be +exhaustively unit-tested on a login node (no GPUs) — **61 unit tests**. + +| File | Role | +|------|------| +| `cvs/cvs/lib/regression_lib.py` | **Pure** A/B detector: gates, percentiles, threshold derivation, report. | +| `cvs/cvs/lib/ci_robustness_lib.py` | **Pure** retry + GPU-cleanup builders/parsers. | +| `cvs/cvs/lib/rccl_lib.py` | Runs one RCCL sweep (`rccl_regression`), `group_rccl_results`, `cleanup_gpus_on_nodes`. | +| `cvs/cvs/tests/rccl/rccl_ab_regression.py` | Pytest orchestration: cleanup → interleaved A/B sweeps (with retry) → analyze. | +| `cvs/cvs/lib/unittests/test_regression_lib.py` | Detector tests incl. Monte-Carlo FP/detection sweeps. | +| `cvs/cvs/lib/unittests/test_ci_robustness_lib.py` | Retry + cleanup tests. | +| `cvs-sbatch/env/thor_rccl_env.sh` | NCCL/IB transport env (cv350 / MI350X + Broadcom Thor RoCE). | +| `cvs-sbatch/config_ab*.json` | A/B run configs. | +| `cvs-sbatch/sbatch/ab_regression.sbatch` | SLURM job (`sp_tests`, 4 nodes / 32 ranks). | +| `cvs-sbatch/run.sh`, `lib/python_env.sh` | Orchestrator: cluster.json gen, per-job uv venv. | + +--- + +## 9. Configuration reference (`rccl` block) + +```jsonc +{ + "rccl": { + "mpi_params": { "no_of_nodes": "4", "no_of_local_ranks": "8", "mpi_pml": "ob1", + "mpi_dir": "/apps/sp/ompi-install", "mpi_oob_port": "10.190.162.57/21" }, + "env_source_script": ".../thor_rccl_env.sh", + "rccl_test_params": { "start_msg_size": "1024", "end_msg_size": "4G", "step_function": "2", + "no_of_iterations": "20", "warmup_iterations": "10", ... }, + "cvs_params": { "nic_model": "thor", "verify_bus_bw": "False", ... }, + + "rccl_collective": ["all_reduce_perf", "reduce_scatter_perf", ...], + "data_types": ["float", "bfloat16"], + "regression": { "NCCL_ALGO": ["Ring"], "NCCL_PROTO": ["Simple"], "NCCL_PXN_DISABLE": ["0","1"] }, + + "gpu_cleanup": { "enabled": true, "kill_gpu_pids": true, "kill_containers": false, "use_sudo": false }, + "retry": { "max_retries": 2, "backoff_sec": 15 }, + + "ab_regression": { + "repeats": 7, + "control_mode": false, // true = reference-vs-itself calibration/stability proof + "safety_factor": 2.0, // thresholds = safety_factor x p95 noise (control mode) + "thresholds": { "small": 0.20, "mid": 0.15, "large": 0.075 }, + "tier_boundaries": { "small_max_bytes": 1048576, "mid_max_bytes": 67108864 }, + "adjacency_min_run": 2, + "min_repeats": 2, + "min_bandwidth_floor": 0.5, + "metric": "busBw", "higher_is_better": true, + "output_dir": "/apps/sp/AIMVT-196/ab_artifacts", + "reference": { "label": "ref", "rccl_tests_dir": ".../reference/.../rccl-tests/build" }, + "candidate": { "label": "cand", "rccl_tests_dir": ".../candidate/.../rccl-tests/build" } + } + } +} +``` + +Notes: +- `librccl.so` is selected automatically by each binary's **rpath** — no + `ld_library_path` needed (but `reference.ld_library_path` / `candidate.ld_library_path` + are supported if a build needs it). +- The test matrix = `rccl_collective` × `data_types` × Cartesian product of `regression` + env vars, each run for A and B × `repeats`. + +--- + +## 10. How to run + +### Local checkout (`/it-share/rccl-ci`) + +Branches: `aimvt-196-rccl-regression-robustness` in both `cvs/` and `cvs-sbatch/`. +Cluster: **amd-tw**, reservation **rccl_dev**, 4 nodes / 32 ranks. + +```bash +# Fast pipeline smoke (control mode, ~30 min): env + orchestration + detector, 0 regressions expected +sbatch /it-share/rccl-ci/sbatch/rccl_ab.sbatch + +# Full-matrix control calibration (reference as both sides). Writes ab_derived_thresholds.json; +# MUST report 0 regressions. +sbatch --export=ALL,CONFIG_JSON=/it-share/rccl-ci/configs/ab_control.json \ + /it-share/rccl-ci/sbatch/rccl_ab.sbatch + +# Real detection (reference vs candidate), using calibrated thresholds. +sbatch --export=ALL,CONFIG_JSON=/it-share/rccl-ci/configs/ab_detect.json \ + /it-share/rccl-ci/sbatch/rccl_ab.sbatch +``` + +**Build paths on this cluster** + +| Side | rccl-tests dir | librccl (via rpath) | +|------|----------------|---------------------| +| reference | `/it-share/rccl-tests/build` | `/it-share/rccl/install/lib` | +| candidate | `/it-share/sp-tests/therock/bin` | `/it-share/sp-tests/therock/lib` | + +**Logs** (under `/it-share/rccl-ci/logs/`): + +- `sp_tests-.out` / `.err` — Slurm capture (tee'd from the job). +- `run__/` — timestamped run bundle: + - `pytest.log` — pytest output + - `slurm.out` / `slurm.err` — copies of the Slurm logs + - `ab_artifacts/` — detector report, thresholds, `rccl_runs.log` +- `latest` — symlink to the most recent `run_*` directory. + +**Artifacts** (also at `ab_artifacts/` during the run): + +- `ab_regression_report.json` — per-key verdicts. +- `ab_derived_thresholds.json` — calibrated thresholds + measured noise (control mode). +- `rccl_runs.log` — clean per-run record: MPI launch command + rccl-tests output. + +Exit code: non-zero (CI fail) if any confirmed regression. + +**Prerequisites on amd-tw** + +- `~/.ssh/cluster_id_ed25519` must exist and authorize SSH to all nodes in the + allocation (auto-detected by `cvs-sbatch/run.sh`). +- RCCL reference/candidate binaries at the paths in `configs/ab_*.json` (see table above). + +### Original cv350 / MI350X cluster (`/apps/sp/AIMVT-196`) + +```bash +# Calibrate + prove stability (reference as both sides). Writes ab_derived_thresholds.json +# and MUST report 0 regressions. +sbatch --export=ALL,CONFIG_JSON=config_ab_full.json \ + /apps/sp/AIMVT-196/cvs-sbatch/sbatch/ab_regression.sbatch # control_mode: true + +# Real detection (reference vs candidate), using calibrated thresholds. +sbatch --export=ALL,CONFIG_JSON=config_ab_full.json \ + /apps/sp/AIMVT-196/cvs-sbatch/sbatch/ab_regression.sbatch # control_mode: false +``` + +- All jobs are named **`sp_tests`**, 4 nodes / 32 ranks, partition `meta64` / `xgmi36`. + +--- + +## 11. Trust model — how we know it's trustworthy + +| Mechanism | What it buys | +|-----------|--------------| +| Paired A/B, interleaved | Cancels common-mode noise; stable even for small messages | +| Triple gate (threshold ∧ separation ∧ adjacency) | A false positive needs three unlikely things at once | +| Median + percentile separation | Resistant to single bad/straggler runs | +| Thresholds derived from measured noise (2× p95) | Bar provably sits above real run-to-run spread | +| `min_bandwidth_floor` → inconclusive | Abstains on the region where no judgment is safe | +| Correct full-key group-by | Compares like-for-like; no hidden/spurious signals | +| Pure, unit-tested core (61 tests) | Deterministic, auditable, regression-proof logic | +| A=A control = 0 regressions | Empirically measures the false-positive rate on real HW | + +### Evidence collected +- **Monte-Carlo (simulated noise):** 0/400 false positives; 400/400 detection of an + injected 15% regression. +- **Real 4-node MI350X control (A=A):** **0 false positives over 920 keys** + (5 collectives × 2 dtypes × PXN {0,1}). +- **Real candidate detection (7.0.2 vs develop):** 106 confirmed regressions with a + coherent, structured signature (selective per collective + PXN-dependent for + all_gather; alltoall clean) — strong evidence of a real, localized change rather + than noise. See `RCCL_REGRESSION_FINDINGS.md`. + +--- + +## 12. Limitations & future work + +- **Global per-tier thresholds** are set by the noisiest collective. A *global* + `large = 7.5%` (driven by alltoall/broadcast noise) means a ~6% large-message + all_reduce regression is missed. **Per-collective thresholds** would recover that + sensitivity without sacrificing stability. +- **Sub-floor tiny messages (1K–64K)** are `inconclusive` (busBw ≈ 0). A + **latency-based comparison** (`metric: "time"`, already supported by the detector) + would extend trustworthy coverage to the smallest sizes, where latency is the + meaningful quantity and pairs just as well. +- **Retry path** is proven by unit tests; a fault-injection run would also exercise a + real transient retry + cleanup cycle on hardware. +- **Single-node** runs hit an OpenMPI intra-node bootstrap issue on this cluster; the + validated path is multi-node under `sbatch` (which is the CI path anyway). +- Periodic **A=A canary** runs in CI are recommended to continuously confirm the + false-positive rate stays 0 as the cluster/software evolves. + +--- + +## 13. One-line summary + +**Trust = paired design to cancel noise + a triple gate and robust statistics to +resist what's left + thresholds calibrated from measured on-hardware noise + an A=A +control that empirically proves zero false positives — all in a pure, unit-tested, +auditable core, wrapped with retry and stale-GPU cleanup for CI resilience.** diff --git a/cvs/lib/rccl_lib.py b/cvs/lib/rccl_lib.py index 3b3e12f86..b0a85f60f 100644 --- a/cvs/lib/rccl_lib.py +++ b/cvs/lib/rccl_lib.py @@ -263,7 +263,17 @@ def _read_json_from_head_node(shdl, head_node, remote_path, log_label): local_path = paths[head_node] log.info('SFTP download succeeded for %s <- %s:%s', log_label, head_node, remote_path) with open(local_path, 'r', encoding='utf-8') as f: - return json.load(f) + content = f.read().strip() + try: + return json.loads(content) + except json.JSONDecodeError as e: + # Legacy rccl-tests (-x) occasionally append duplicate JSON blobs; + # accept the first valid value so paired A/B runs can proceed. + if 'Extra data' in str(e): + obj, _ = json.JSONDecoder().raw_decode(content) + log.warning('Parsed first JSON value only for %s; trailing data ignored', log_label) + return obj + raise def is_ucx_available_in_mpi(shdl, mpi_path, head_node): @@ -817,6 +827,16 @@ def rccl_regression( rccl_result_file = cvs_params.get('rccl_result_file', '/tmp/rccl_result_output.json') cvs_exec_timeout = int(cvs_params.get('cvs_exec_timeout', 2400)) + # Per-collective wall-clock timeout (seconds). A hung collective (e.g. an + # alltoall MPI/kernel deadlock) is NOT stopped by the parallel-ssh read + # timeout — that only abandons the SSH read and leaves mpirun + ranks alive, + # holding GPUs and wedging the whole CI. We therefore wrap the REMOTE mpirun + # in coreutils `timeout`, so the compute node kills the process tree itself. + # On expiry mpirun exits 124, the result file is absent/partial, and the + # caller's retry/cleanup path takes over (instead of hanging indefinitely). + # Configurable via rccl_test_params.per_collective_timeout_sec (0 disables). + per_collective_timeout = int(rccl_test_params.get('per_collective_timeout_sec', 900)) + # Detect which output file argument is supported by the RCCL test binary rccl_test_binary_path = f'{rccl_tests_dir}/{test_name}' output_flag = detect_rccl_output_flag(shdl, rccl_test_binary_path, head_node) @@ -854,11 +874,18 @@ def rccl_regression( if env_overrides: env_override_params = ' '.join([f'-x {k}={v}' for k, v in env_overrides.items()]) + # Per-collective timeout wrapper around the REMOTE mpirun. --kill-after sends + # SIGKILL if mpirun ignores the initial SIGTERM (a wedged PRRTE often does). + # 0/negative disables the wrapper. + timeout_prefix = '' + if per_collective_timeout > 0: + timeout_prefix = f'timeout --signal=TERM --kill-after=30s {per_collective_timeout}s ' + # Build mpirun command. # plm_rsh_args disables interactive host-key prompts so PRRTE can ssh-launch # ranks on the other allocated nodes non-interactively (required for multi-node # runs; harmless single-node). Mirrors the older working recipe. - cmd = f'''{mpi_dir}/bin/mpirun \ + cmd = f'''{timeout_prefix}{mpi_dir}/bin/mpirun \ --allow-run-as-root \ -np {no_of_global_ranks} \ --hostfile /tmp/rccl_hosts_file.txt \ @@ -876,8 +903,14 @@ def rccl_regression( log.info("%s", cmd) log.info('%%%%%%%%%%%%%%%%') + # Let the remote coreutils `timeout` fire FIRST (clean kill + 124), so keep the + # SSH read timeout comfortably larger than the per-collective budget. + exec_timeout = cvs_exec_timeout + if per_collective_timeout > 0: + exec_timeout = max(cvs_exec_timeout, per_collective_timeout + 120) + try: - out_dict = shdl.exec(cmd, timeout=cvs_exec_timeout) + out_dict = shdl.exec(cmd, timeout=exec_timeout) output = out_dict[head_node] scan_rccl_logs(output) # Write a clean, reproducible record (launch command + rccl-tests output) @@ -1019,6 +1052,17 @@ def rccl_perf( rccl_result_file = cvs_params.get('rccl_result_file', '/tmp/rccl_result_output.json') cvs_exec_timeout = int(cvs_params.get('cvs_exec_timeout', 2400)) + # Per-collective wall-clock timeout — see rccl_regression() for rationale. The + # remote mpirun is wrapped in coreutils `timeout` so a hung collective is killed + # on the node instead of leaking ranks/GPUs. 0 disables. + per_collective_timeout = int(rccl_test_params.get('per_collective_timeout_sec', 900)) + exec_timeout = cvs_exec_timeout + if per_collective_timeout > 0: + exec_timeout = max(cvs_exec_timeout, per_collective_timeout + 120) + timeout_prefix = '' + if per_collective_timeout > 0: + timeout_prefix = f'timeout --signal=TERM --kill-after=30s {per_collective_timeout}s ' + all_raw_results = [] all_validated_results = [] base_path = Path(rccl_result_file) @@ -1050,7 +1094,7 @@ def rccl_perf( test_cmd = f'bash -c "{test_cmd}"' # Build mpirun command - cmd = f'''{mpi_dir}/bin/mpirun --np {no_of_global_ranks} \ + cmd = f'''{timeout_prefix}{mpi_dir}/bin/mpirun --np {no_of_global_ranks} \ --allow-run-as-root \ --hostfile /tmp/rccl_hosts_file.txt \ --bind-to numa \ @@ -1066,7 +1110,7 @@ def rccl_perf( log.info("%s", cmd) log.info('%%%%%%%%%%%%%%%%') try: - out_dict = shdl.exec(cmd, timeout=cvs_exec_timeout) + out_dict = shdl.exec(cmd, timeout=exec_timeout) output = out_dict[head_node] # print(output) scan_rccl_logs(output) diff --git a/cvs/lib/unittests/test_ab_parametrize.py b/cvs/lib/unittests/test_ab_parametrize.py new file mode 100644 index 000000000..d10238dee --- /dev/null +++ b/cvs/lib/unittests/test_ab_parametrize.py @@ -0,0 +1,168 @@ +""" +Unit tests for the A/B test parametrization hook (pytest_generate_tests). + +These verify the decoupling of *perf-regression* axes (collective x dtype x size) +from the optional NCCL *knob* matrix (`regression`): + + * With no `regression` block, every collective/dtype is still parametrized and + each runs once under the production env (a single empty knob override). + * With a `regression` block, the NCCL knob matrix is expanded as a Cartesian + product (extra coverage) - the legacy behaviour. +""" + +import json +import os +import tempfile +import unittest + +import cvs.tests.rccl.rccl_ab_regression as ab + + +class _FakeConfig: + def __init__(self, config_file): + self._config_file = config_file + + def getoption(self, name): + if name == "config_file": + return self._config_file + return None + + +class _FakeMetafunc: + """Minimal stand-in for pytest's Metafunc, capturing parametrize() calls.""" + + def __init__(self, config_file, fixturenames): + self.config = _FakeConfig(config_file) + self.fixturenames = fixturenames + self.calls = {} # argname -> {"argvalues": [...], "ids": [...]} + + def parametrize(self, argname, argvalues, ids=None): + self.calls[argname] = {"argvalues": list(argvalues), "ids": list(ids) if ids else None} + + +def _write_cfg(tmp, rccl_block): + path = os.path.join(tmp, "cfg.json") + with open(path, "w") as fp: + json.dump({"rccl": rccl_block}, fp) + return path + + +FIXTURES = ["rccl_collective", "regression_params", "data_type"] + + +class TestAbParametrize(unittest.TestCase): + def test_no_regression_block_runs_collectives_under_default_env(self): + with tempfile.TemporaryDirectory() as tmp: + cfg = _write_cfg(tmp, { + "rccl_collective": ["all_reduce_perf", "all_gather_perf"], + "data_types": ["float", "bfloat16"], + # no "regression" key at all + }) + mf = _FakeMetafunc(cfg, FIXTURES) + ab.pytest_generate_tests(mf) + + self.assertEqual(mf.calls["rccl_collective"]["argvalues"], + ["all_reduce_perf", "all_gather_perf"]) + # Exactly one knob combo: the production-env default. + self.assertEqual(mf.calls["regression_params"]["argvalues"], [{}]) + self.assertEqual(mf.calls["regression_params"]["ids"], ["default"]) + self.assertEqual(mf.calls["data_type"]["argvalues"], ["float", "bfloat16"]) + + def test_empty_regression_block_is_treated_as_default(self): + with tempfile.TemporaryDirectory() as tmp: + cfg = _write_cfg(tmp, { + "rccl_collective": ["all_reduce_perf"], + "regression": {}, + }) + mf = _FakeMetafunc(cfg, FIXTURES) + ab.pytest_generate_tests(mf) + + self.assertEqual(mf.calls["regression_params"]["argvalues"], [{}]) + self.assertEqual(mf.calls["regression_params"]["ids"], ["default"]) + + def test_regression_block_expands_knob_matrix(self): + with tempfile.TemporaryDirectory() as tmp: + cfg = _write_cfg(tmp, { + "rccl_collective": ["all_reduce_perf"], + "regression": {"NCCL_PXN_DISABLE": ["0", "1"]}, + }) + mf = _FakeMetafunc(cfg, FIXTURES) + ab.pytest_generate_tests(mf) + + self.assertEqual( + mf.calls["regression_params"]["argvalues"], + [{"NCCL_PXN_DISABLE": "0"}, {"NCCL_PXN_DISABLE": "1"}], + ) + self.assertEqual( + mf.calls["regression_params"]["ids"], + ["NCCL_PXN_DISABLE=0", "NCCL_PXN_DISABLE=1"], + ) + + def test_cartesian_product_of_two_knobs(self): + with tempfile.TemporaryDirectory() as tmp: + cfg = _write_cfg(tmp, { + "rccl_collective": ["all_reduce_perf"], + "regression": {"NCCL_ALGO": ["Ring"], "NCCL_PXN_DISABLE": ["0", "1"]}, + }) + mf = _FakeMetafunc(cfg, FIXTURES) + ab.pytest_generate_tests(mf) + + self.assertEqual(len(mf.calls["regression_params"]["argvalues"]), 2) + for combo in mf.calls["regression_params"]["argvalues"]: + self.assertEqual(combo["NCCL_ALGO"], "Ring") + + +class TestResolveDetectThresholds(unittest.TestCase): + CONFIG_THR = {"small": 0.20, "mid": 0.15, "large": 0.075} + DERIVED_THR = {"small": 0.10, "mid": 0.05, "large": 0.03} + + def _write_derived(self, tmp, payload): + with open(os.path.join(tmp, "ab_derived_thresholds.json"), "w") as fp: + json.dump(payload, fp) + + def test_derived_file_overrides_config(self): + with tempfile.TemporaryDirectory() as tmp: + self._write_derived(tmp, {"thresholds": self.DERIVED_THR}) + out = ab._resolve_detect_thresholds( + {"thresholds": self.CONFIG_THR}, {}, tmp) + self.assertEqual(out["thresholds"], self.DERIVED_THR) + + def test_missing_file_keeps_config(self): + with tempfile.TemporaryDirectory() as tmp: + out = ab._resolve_detect_thresholds( + {"thresholds": self.CONFIG_THR}, {}, tmp) + self.assertEqual(out["thresholds"], self.CONFIG_THR) + + def test_opt_out_keeps_config_even_if_file_present(self): + with tempfile.TemporaryDirectory() as tmp: + self._write_derived(tmp, {"thresholds": self.DERIVED_THR}) + out = ab._resolve_detect_thresholds( + {"thresholds": self.CONFIG_THR}, + {"use_derived_thresholds": False}, tmp) + self.assertEqual(out["thresholds"], self.CONFIG_THR) + + def test_corrupt_file_keeps_config(self): + with tempfile.TemporaryDirectory() as tmp: + with open(os.path.join(tmp, "ab_derived_thresholds.json"), "w") as fp: + fp.write("{ not json") + out = ab._resolve_detect_thresholds( + {"thresholds": self.CONFIG_THR}, {}, tmp) + self.assertEqual(out["thresholds"], self.CONFIG_THR) + + def test_file_without_thresholds_key_keeps_config(self): + with tempfile.TemporaryDirectory() as tmp: + self._write_derived(tmp, {"noise": {}}) + out = ab._resolve_detect_thresholds( + {"thresholds": self.CONFIG_THR}, {}, tmp) + self.assertEqual(out["thresholds"], self.CONFIG_THR) + + def test_does_not_mutate_input(self): + with tempfile.TemporaryDirectory() as tmp: + self._write_derived(tmp, {"thresholds": self.DERIVED_THR}) + original = {"thresholds": self.CONFIG_THR} + ab._resolve_detect_thresholds(original, {}, tmp) + self.assertEqual(original["thresholds"], self.CONFIG_THR) + + +if __name__ == "__main__": + unittest.main() diff --git a/cvs/tests/rccl/rccl_ab_regression.py b/cvs/tests/rccl/rccl_ab_regression.py index dc246f520..808def1f4 100644 --- a/cvs/tests/rccl/rccl_ab_regression.py +++ b/cvs/tests/rccl/rccl_ab_regression.py @@ -135,13 +135,15 @@ def pytest_generate_tests(metafunc): with open(config_file) as fp: cfg = json.load(fp) rccl = cfg.get("rccl", {}) + # The NCCL knob matrix (`regression`) is OPTIONAL for the A/B perf-regression + # test. Perf regression is a question about collective x dtype x message size + # under the production env; sweeping NCCL knobs (PXN, channels, ALGO/PROTO, + # P2P, ...) is extra *coverage*, not a prerequisite for a verdict. When absent + # we run each collective once under the env_source_script defaults. regression = dict(rccl.get("regression", {})) - if not regression: - log.error("No regression object found in config - required for A/B parametrization") - return # Paired channel handling (min/max kept paired, not Cartesian) - identical to - # the single-sided regression test. + # the single-sided regression test. No-op when `regression` is empty. has_min = "NCCL_MIN_NCHANNELS" in regression has_max = "NCCL_MAX_NCHANNELS" in regression if has_min != has_max: @@ -162,25 +164,33 @@ def pytest_generate_tests(metafunc): if isinstance(value, list) and value: env_axes.append((key, value)) - if env_axes and "rccl_collective" in metafunc.fixturenames: + if "rccl_collective" in metafunc.fixturenames: rccl_collective_list = rccl.get("rccl_collective", ["all_reduce_perf"]) - env_fixture_names = [name for name, _ in env_axes] - env_domains = [dict(env_axes)[name] for name in env_fixture_names] env_params, env_ids = [], [] - channel_fixture_names = [] - if paired_channels is not None: - channel_fixture_names = ["NCCL_MIN_NCHANNELS", "NCCL_MAX_NCHANNELS"] - env_domains.append(paired_channels) + if env_axes: + # Optional NCCL knob matrix: Cartesian product over each env axis. + env_fixture_names = [name for name, _ in env_axes] + env_domains = [dict(env_axes)[name] for name in env_fixture_names] - for env_combo in itertools.product(*env_domains): - env_dict = dict(zip(env_fixture_names + channel_fixture_names, env_combo)) + channel_fixture_names = [] if paired_channels is not None: - min_ch, max_ch = env_dict.pop("NCCL_MIN_NCHANNELS") - env_dict["NCCL_MIN_NCHANNELS"] = min_ch - env_dict["NCCL_MAX_NCHANNELS"] = max_ch - env_params.append(env_dict) - env_ids.append("|".join(f"{k}={v}" for k, v in env_dict.items())) + channel_fixture_names = ["NCCL_MIN_NCHANNELS", "NCCL_MAX_NCHANNELS"] + env_domains.append(paired_channels) + + for env_combo in itertools.product(*env_domains): + env_dict = dict(zip(env_fixture_names + channel_fixture_names, env_combo)) + if paired_channels is not None: + min_ch, max_ch = env_dict.pop("NCCL_MIN_NCHANNELS") + env_dict["NCCL_MIN_NCHANNELS"] = min_ch + env_dict["NCCL_MAX_NCHANNELS"] = max_ch + env_params.append(env_dict) + env_ids.append("|".join(f"{k}={v}" for k, v in env_dict.items())) + else: + # Default perf-regression path: one run per collective under the + # production env (env_source_script), no NCCL knob override. + env_params = [{}] + env_ids = ["default"] metafunc.parametrize("rccl_collective", rccl_collective_list) metafunc.parametrize("regression_params", env_params, ids=env_ids) @@ -194,6 +204,42 @@ def pytest_generate_tests(metafunc): # --------------------------------------------------------------------------- # # Helpers # --------------------------------------------------------------------------- # +def _resolve_detect_thresholds(detector_overrides, ab_cfg, out_dir): + """ + In detect mode, override config thresholds with the ones calibrated on THIS + hardware (``ab_derived_thresholds.json`` written by a prior control run), so the + gate never silently runs on stale numbers. Pure/testable: returns a (possibly + new) detector_overrides dict and never raises. + + Escape hatch: ``ab_regression.use_derived_thresholds: false`` forces the config + thresholds. If the derived file is missing/unreadable, config thresholds stand. + """ + if not ab_cfg.get('use_derived_thresholds', True): + return detector_overrides + + derived_path = os.path.join(out_dir, 'ab_derived_thresholds.json') + cfg_thr = detector_overrides.get("thresholds") + try: + with open(derived_path) as fp: + derived_thr = (json.load(fp) or {}).get('thresholds') + except FileNotFoundError: + log.warning("No calibrated thresholds at %s; using config thresholds %s. " + "Run a control-mode calibration first.", derived_path, cfg_thr) + return detector_overrides + except ValueError as exc: + log.warning("Could not parse %s (%s); using config thresholds %s.", + derived_path, exc, cfg_thr) + return detector_overrides + + if derived_thr: + log.info("Using calibrated thresholds from %s: %s (overriding config)", + derived_path, derived_thr) + return {**detector_overrides, "thresholds": derived_thr} + + log.warning("%s has no 'thresholds'; using config thresholds %s.", derived_path, cfg_thr) + return detector_overrides + + def _side_params(base_rccl_test_params, side_cfg, data_type=None): """Build a per-side copy of rccl_test_params with the build's tests dir / lib path.""" params = copy.deepcopy(base_rccl_test_params) @@ -314,6 +360,19 @@ def test_ab_pair(phdl, shdl, cluster_dict, config_dict, rccl_collective, regress globals.error_list = [] ab_cfg = config_dict.get('ab_regression', {}) + + # Excluded (collective, dtype) combinations. Used for combos that are broken + # UPSTREAM (e.g. a collective that fails rccl-tests data verification on this + # ROCm/RCCL revision) — running them would only exhaust retries and then HARD + # FAIL the whole gate job, masking the real verdict. We pytest.skip them so the + # gate stays green/meaningful for the working matrix; revisit when upstream + # fixes the combo. Each entry is a [collective, data_type] pair, e.g. + # "skip_keys": [["alltoall_perf", "bfloat16"]] + skip_keys = {(str(c), str(d)) for c, d in ab_cfg.get('skip_keys', [])} + if (str(rccl_collective), str(data_type)) in skip_keys: + pytest.skip(f"{rccl_collective}/{data_type} excluded via ab_regression.skip_keys " + f"(known upstream issue; not a gate failure)") + repeats = int(ab_cfg.get('repeats', 7)) control_mode = bool(ab_cfg.get('control_mode', False)) @@ -326,7 +385,7 @@ def test_ab_pair(phdl, shdl, cluster_dict, config_dict, rccl_collective, regress reference_cfg = {**reference_cfg, "label": reference_cfg.get("label", "ref")} candidate_cfg = {**candidate_cfg, "label": candidate_cfg.get("label", "cand")} - params_str = ' '.join(f'{k}={v}' for k, v in regression_params.items()) + params_str = ' '.join(f'{k}={v}' for k, v in regression_params.items()) or 'default' group_key = f'{rccl_collective}-d={data_type}-{params_str}' ab_runs.setdefault(group_key, {"a": [], "b": []}) @@ -382,6 +441,10 @@ def test_ab_analyze(request, config_dict): json.dump(derived, fp, indent=2) # Apply derived thresholds for the (sanity) detection below. detector_overrides = {**detector_overrides, "thresholds": derived["thresholds"]} + else: + # Detect mode: prefer thresholds calibrated on THIS hardware (written by a + # prior control run) over the potentially-stale values baked into the config. + detector_overrides = _resolve_detect_thresholds(detector_overrides, ab_cfg, out_dir) for group_key, runs in ab_runs.items(): report = regression_lib.detect_regressions(runs["a"], runs["b"], config=detector_overrides or None) diff --git a/uv.lock b/uv.lock new file mode 100644 index 000000000..a02a6a37f --- /dev/null +++ b/uv.lock @@ -0,0 +1,3 @@ +version = 1 +revision = 3 +requires-python = ">=3.10" From 9e449a05767c5f53c165da397658181edbb306bb Mon Sep 17 00:00:00 2001 From: surya periaswamy Date: Fri, 17 Jul 2026 19:32:42 +0000 Subject: [PATCH 05/17] Add rccl_perf_gate: Slurm glue for the RCCL A/B perf regression CI gate Relocates the RCCL CI gate's build/submit/poll/report scripts here from an untracked NFS location, addressing review feedback on rocm-systems#8351 that these scripts weren't version controlled. Placed under cvs (rather than a new repo, rocm-systems, or cvs-sbatch) since these bash wrappers around sbatch/squeue are a stopgap for scheduler-submission capability CVS is expected to gain natively; see ci/rccl_perf_gate/README.md. All scripts honor an RCCL_CI_ROOT override so they aren't tied to one cluster's NFS layout. --- ci/rccl_perf_gate/README.md | 27 ++++ ci/rccl_perf_gate/format_report.py | 172 +++++++++++++++++++++ ci/rccl_perf_gate/sbatch/rccl_ab.sbatch | 46 ++++++ ci/rccl_perf_gate/sbatch/rccl_build.sbatch | 38 +++++ ci/rccl_perf_gate/sbatch/run_rccl_ab.sh | 142 +++++++++++++++++ ci/rccl_perf_gate/sbatch/run_rccl_build.sh | 33 ++++ ci/rccl_perf_gate/submit_and_poll.sh | 94 +++++++++++ 7 files changed, 552 insertions(+) create mode 100644 ci/rccl_perf_gate/README.md create mode 100644 ci/rccl_perf_gate/format_report.py create mode 100644 ci/rccl_perf_gate/sbatch/rccl_ab.sbatch create mode 100755 ci/rccl_perf_gate/sbatch/rccl_build.sbatch create mode 100755 ci/rccl_perf_gate/sbatch/run_rccl_ab.sh create mode 100755 ci/rccl_perf_gate/sbatch/run_rccl_build.sh create mode 100755 ci/rccl_perf_gate/submit_and_poll.sh diff --git a/ci/rccl_perf_gate/README.md b/ci/rccl_perf_gate/README.md new file mode 100644 index 000000000..502cb9a7e --- /dev/null +++ b/ci/rccl_perf_gate/README.md @@ -0,0 +1,27 @@ +# rccl_perf_gate + +Slurm submission/polling/reporting glue for the RCCL paired A/B performance +regression gate used by `ROCm/rocm-systems`'s +[`rccl_perf_regression.yml`](https://github.com/ROCm/rocm-systems/blob/main/.github/workflows/rccl_perf_regression.yml) +GitHub Actions workflow. + +The workflow's self-hosted runner invokes these scripts directly: + +- `sbatch/rccl_build.sbatch`, `sbatch/run_rccl_build.sh` — build RCCL (via + `cvs-sbatch`) as a Slurm job. +- `submit_and_poll.sh`, `sbatch/rccl_ab.sbatch`, `sbatch/run_rccl_ab.sh` — + submit the paired A/B regression job (`cvs/tests/rccl/rccl_ab_regression.py`), + poll it to completion, and map its exit code to a CI-gatable result. +- `format_report.py` — render the A/B run's JSON result into a Markdown + summary for the workflow's job summary / PR comment. + +All scripts honor an `RCCL_CI_ROOT` env override (default `/it-share/rccl-ci`) +so they aren't tied to one cluster's NFS layout. + +## Status + +This is a stopgap. It exists because CVS does not yet submit and manage Slurm +(or Kubernetes) jobs natively — these scripts are thin bash wrappers around +`sbatch`/`squeue` bridging that gap. Once CVS gains native scheduler +integration, this directory should be retired in favor of driving the A/B +regression test directly through CVS. diff --git a/ci/rccl_perf_gate/format_report.py b/ci/rccl_perf_gate/format_report.py new file mode 100644 index 000000000..80b8b0a98 --- /dev/null +++ b/ci/rccl_perf_gate/format_report.py @@ -0,0 +1,172 @@ +#!/usr/bin/env python3 +"""Render an RCCL A/B regression report (ab_regression_report.json) as Markdown. + +Consumes the JSON written by ``cvs/tests/rccl/rccl_ab_regression.py`` +(``{"control_mode": bool, "reports": {group_key: }}``) +and emits a GitHub-flavoured Markdown summary suitable for a PR comment or a +GitHub Actions step summary. + +Exit code mirrors the gate verdict so the same invocation can drive a check: + 0 = PASS (no confirmed regressions) + 1 = REGRESSION DETECTED (detect mode) / detector unstable (control mode) +Use ``--no-exit-code`` to always exit 0 (e.g. when only rendering). +""" + +import argparse +import json +import sys +from pathlib import Path + + +def _fmt_size(n): + """Human-readable byte size (1024-based), e.g. 1024 -> '1K', 4294967296 -> '4G'.""" + try: + n = int(n) + except (TypeError, ValueError): + return str(n) + for unit in ("", "K", "M", "G", "T"): + if abs(n) < 1024 or unit == "T": + return f"{n}{unit}" if unit == "" else f"{n:.0f}{unit}" + n /= 1024.0 + return str(n) + + +def _thresholds_line(reports): + """Pull the per-tier thresholds from the first report (identical across groups).""" + for rep in reports.values(): + thr = rep.get("config", {}).get("thresholds") + if thr: + return ( + f"small {thr.get('small', 0) * 100:.1f}% · " + f"mid {thr.get('mid', 0) * 100:.1f}% · " + f"large {thr.get('large', 0) * 100:.1f}%" + ) + return "n/a" + + +def _collect(reports): + """Aggregate counts and flatten confirmed regressions across all groups.""" + totals = {"keys": 0, "regressions": 0, "inconclusive": 0, "candidates": 0} + regressions = [] + for group_key, rep in reports.items(): + s = rep.get("summary", {}) + totals["keys"] += s.get("keys_compared", 0) + totals["regressions"] += s.get("regressions", 0) + totals["inconclusive"] += s.get("inconclusive", 0) + totals["candidates"] += s.get("candidates", 0) + for v in rep.get("regressions", []): + k = v.get("key", {}) + regressions.append( + { + "collective": k.get("name", "?"), + "dtype": k.get("type", "?"), + "size": k.get("size", 0), + "a_med": v.get("a", {}).get("median", 0.0), + "b_med": v.get("b", {}).get("median", 0.0), + "drop": v.get("rel_drop", 0.0), + "thr": v.get("threshold", 0.0), + } + ) + regressions.sort(key=lambda r: (r["collective"], str(r["dtype"]), r["size"])) + return totals, regressions + + +def render(report_data, title="RCCL Perf-Regression Gate"): + """Return a Markdown string for the given parsed report JSON.""" + control_mode = bool(report_data.get("control_mode", False)) + reports = report_data.get("reports", {}) + totals, regressions = _collect(reports) + has_regression = totals["regressions"] > 0 + + lines = [] + if control_mode: + # In a control (A=A) run, any regression is a false positive => gate broken. + verdict = "❌ DETECTOR UNSTABLE" if has_regression else "✅ STABLE (0 false positives)" + lines.append(f"## {title}: {verdict}") + lines.append("") + lines.append("**Mode:** calibration / control (A=A — same build both sides)") + else: + verdict = "❌ REGRESSION DETECTED" if has_regression else "✅ PASS" + lines.append(f"## {title}: {verdict}") + lines.append("") + lines.append("**Mode:** detect (reference vs candidate)") + + lines.append(f"**Thresholds (per-tier, calibrated):** {_thresholds_line(reports)}") + lines.append( + f"**Keys compared:** {totals['keys']} · " + f"**Confirmed regressions:** {totals['regressions']} · " + f"**Inconclusive:** {totals['inconclusive']}" + ) + lines.append("") + + if regressions: + lines.append(f"### Confirmed regressions ({len(regressions)})") + lines.append("") + lines.append("| collective | dtype | size | A (ref) GB/s | B (cand) GB/s | drop % | thr % |") + lines.append("|---|---|---:|---:|---:|---:|---:|") + for r in regressions: + lines.append( + f"| {r['collective']} | {r['dtype']} | {_fmt_size(r['size'])} " + f"| {r['a_med']:.2f} | {r['b_med']:.2f} " + f"| {r['drop'] * 100:.1f} | {r['thr'] * 100:.1f} |" + ) + lines.append("") + + # Per-group breakdown (collapsed) so reviewers can see coverage / inconclusive spread. + lines.append("
Per-collective breakdown") + lines.append("") + lines.append("| group | keys | regressions | inconclusive |") + lines.append("|---|---:|---:|---:|") + for group_key, rep in sorted(reports.items()): + s = rep.get("summary", {}) + lines.append( + f"| {group_key} | {s.get('keys_compared', 0)} " + f"| {s.get('regressions', 0)} | {s.get('inconclusive', 0)} |" + ) + lines.append("") + lines.append("
") + lines.append("") + return "\n".join(lines), has_regression, control_mode + + +def main(argv=None): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "report", + nargs="?", + default="/it-share/rccl-ci/ab_artifacts/ab_regression_report.json", + help="Path to ab_regression_report.json", + ) + parser.add_argument("-o", "--output", help="Write Markdown here (default: stdout)") + parser.add_argument("--title", default="RCCL Perf-Regression Gate", help="Heading title") + parser.add_argument( + "--no-exit-code", + action="store_true", + help="Always exit 0 (do not map verdict to exit code)", + ) + args = parser.parse_args(argv) + + path = Path(args.report) + try: + report_data = json.loads(path.read_text()) + except FileNotFoundError: + print(f"error: report not found: {path}", file=sys.stderr) + return 2 + except ValueError as exc: + print(f"error: could not parse {path}: {exc}", file=sys.stderr) + return 2 + + markdown, has_regression, _control = render(report_data, title=args.title) + + if args.output: + Path(args.output).write_text(markdown) + else: + print(markdown) + + if args.no_exit_code: + return 0 + return 1 if has_regression else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/ci/rccl_perf_gate/sbatch/rccl_ab.sbatch b/ci/rccl_perf_gate/sbatch/rccl_ab.sbatch new file mode 100644 index 000000000..e6986e276 --- /dev/null +++ b/ci/rccl_perf_gate/sbatch/rccl_ab.sbatch @@ -0,0 +1,46 @@ +#!/usr/bin/env bash + +############################################################## +# RCCL paired A/B regression CI entry point. +# +# Submit from anywhere: +# sbatch /it-share/rccl-ci/cvs/ci/rccl_perf_gate/sbatch/rccl_ab.sbatch +# +# Override the benchmark config (default: fast control / robustness): +# sbatch --export=ALL,CONFIG_JSON=/it-share/rccl-ci/configs/ab_control.json \ +# /it-share/rccl-ci/cvs/ci/rccl_perf_gate/sbatch/rccl_ab.sbatch +# +# Presets: +# configs/ab_robustness.json - fast 4-node control (pipeline smoke) +# configs/ab_control.json - full-matrix control + threshold calibration +# configs/ab_detect.json - reference vs candidate detection +############################################################## + +#SBATCH --job-name=sp_tests +#SBATCH --partition=amd-tw +# reservation passed via CLI --reservation= when SLURM_RESERVATION is set +#SBATCH --account=amd-tw +#SBATCH --nodes=4 +#SBATCH --ntasks=4 +#SBATCH --ntasks-per-node=1 +#SBATCH --cpus-per-task=8 +#SBATCH --exclusive +# Pin to a fixed 4-node set so calibration and detection share identical hardware +# (keeps derived thresholds valid across runs). Override with --nodelist at submit. +#SBATCH --nodelist=mia1-p01-g[22,26,28,32] +#SBATCH --time=08:00:00 +#SBATCH --chdir=/it-share/rccl-ci + +#SBATCH --output=/it-share/rccl-ci/logs/sp_tests-%j.out +#SBATCH --error=/it-share/rccl-ci/logs/sp_tests-%j.err + +set -euo pipefail + +# Only the batch-script process on the head node runs the orchestrator; other +# allocated tasks exit immediately so we keep a true 4-node exclusive job. +if [[ "${SLURM_PROCID:-0}" -ne 0 ]]; then + exit 0 +fi + +readonly RCCL_CI_ROOT="${RCCL_CI_ROOT:-/it-share/rccl-ci}" +exec bash "${RCCL_CI_ROOT}/cvs/ci/rccl_perf_gate/sbatch/run_rccl_ab.sh" diff --git a/ci/rccl_perf_gate/sbatch/rccl_build.sbatch b/ci/rccl_perf_gate/sbatch/rccl_build.sbatch new file mode 100755 index 000000000..c0cb80824 --- /dev/null +++ b/ci/rccl_perf_gate/sbatch/rccl_build.sbatch @@ -0,0 +1,38 @@ +#!/usr/bin/env bash + +############################################################## +# RCCL build-only Slurm entry point. +# +# Builds reference + candidate librccl.so on a single node, independent of the +# 4-node detection allocation (rccl_ab.sbatch). Split out because the build is +# a single-node CPU compile with no RDMA/MPI involved -- holding all 4 pinned +# nodes exclusively for the ~15-20min build duration wastes 3 of them, and +# ties up the pinned-node concurrency lock longer than the detection step +# actually needs it. +# +# Submit from anywhere: +# sbatch --export=ALL,CANDIDATE_SRC=...,BASE_REF=...,CONFIG_JSON=... \ +# /it-share/rccl-ci/cvs/ci/rccl_perf_gate/sbatch/rccl_build.sbatch +############################################################## + +#SBATCH --job-name=sp_build +#SBATCH --partition=amd-tw +# reservation passed via CLI --reservation= when SLURM_RESERVATION is set +#SBATCH --account=amd-tw +#SBATCH --nodes=1 +#SBATCH --ntasks=1 +#SBATCH --exclusive +# Build is single-node CPU compile only (no RDMA/MPI) -- pin to one of the +# reserved nodes. Override with --nodelist at submit if g28 is unavailable +# (e.g. mia1-p01-g32). +#SBATCH --nodelist=mia1-p01-g28 +#SBATCH --time=01:00:00 +#SBATCH --chdir=/it-share/rccl-ci + +#SBATCH --output=/it-share/rccl-ci/logs/sp_build-%j.out +#SBATCH --error=/it-share/rccl-ci/logs/sp_build-%j.err + +set -euo pipefail + +readonly RCCL_CI_ROOT="${RCCL_CI_ROOT:-/it-share/rccl-ci}" +exec bash "${RCCL_CI_ROOT}/cvs/ci/rccl_perf_gate/sbatch/run_rccl_build.sh" diff --git a/ci/rccl_perf_gate/sbatch/run_rccl_ab.sh b/ci/rccl_perf_gate/sbatch/run_rccl_ab.sh new file mode 100755 index 000000000..63e30fcd9 --- /dev/null +++ b/ci/rccl_perf_gate/sbatch/run_rccl_ab.sh @@ -0,0 +1,142 @@ +#!/usr/bin/env bash +# Shared RCCL A/B regression runner (no #SBATCH directives). +# Invoked by sbatch/rccl_ab.sbatch or manually via srun inside an allocation. + +set -euo pipefail + +readonly RCCL_CI_ROOT="${RCCL_CI_ROOT:-/it-share/rccl-ci}" +readonly TIMESTAMP="$(date +%Y%m%d_%H%M%S)" +readonly JOB_TAG="${SLURM_JOB_ID:-local}" + +# Orchestrator must run on the login/submit host and SSH to compute nodes. +# Running inside `srun` on a compute node ties the pytest process to that Slurm +# step; a node blip or heavy MPI load kills the whole pipeline before artifacts +# are written. Refuse that launch mode with a clear message. +if [[ -n "${SLURM_JOB_ID:-}" && -n "${SLURM_STEP_ID:-}" ]]; then + echo "[ERROR] Do not launch run_rccl_ab.sh via srun on a compute node." >&2 + echo " Use sbatch, or from the login node with the wrap-job env:" >&2 + echo " export SLURM_JOB_ID= SLURM_NODELIST='' SLURM_NNODES=4" >&2 + echo " CONFIG_JSON=... bash ${RCCL_CI_ROOT}/sbatch/run_rccl_ab.sh" >&2 + exit 1 +fi + +export RUN_LOG_DIR="${RCCL_CI_ROOT}/logs/run_${TIMESTAMP}_${JOB_TAG}" +mkdir -p "${RUN_LOG_DIR}" + +readonly SLURM_LOG_DIR="${RCCL_CI_ROOT}/logs" +mkdir -p "${SLURM_LOG_DIR}" + +export CVS_DIR="${RCCL_CI_ROOT}/cvs" +export CVS_SBATCH_DIR="${RCCL_CI_ROOT}/cvs-sbatch" +export SKIP_CVS_SETUP=1 +export CONFIG_JSON="${CONFIG_JSON:-${RCCL_CI_ROOT}/configs/ab_robustness.json}" +# Normalise to an absolute path. run.sh runs from cvs-sbatch/ and resolves the +# config with realpath, so a path relative to RCCL_CI_ROOT (or the submit cwd) +# would otherwise fail there. Accept either form. +if [[ "${CONFIG_JSON}" != /* ]]; then + if [[ -f "${RCCL_CI_ROOT}/${CONFIG_JSON}" ]]; then + CONFIG_JSON="${RCCL_CI_ROOT}/${CONFIG_JSON}" + elif [[ -f "${CONFIG_JSON}" ]]; then + CONFIG_JSON="$(cd "$(dirname "${CONFIG_JSON}")" && pwd)/$(basename "${CONFIG_JSON}")" + fi + export CONFIG_JSON +fi +[[ -f "${CONFIG_JSON}" ]] || { echo "[ERROR] CONFIG_JSON not found: ${CONFIG_JSON}" >&2; exit 1; } +export TEST_PATH="${TEST_PATH:-./cvs/tests/rccl/rccl_ab_regression.py}" +export LOG_FILE="${RUN_LOG_DIR}/pytest.log" + +# Seed librocm_smi64.so.1 compat symlink on every allocated node before MPI. +if [[ -n "${SLURM_NODELIST:-}" ]]; then + echo "[INFO] Seeding /tmp/rocm_smi_fix on allocation nodes..." + while IFS= read -r _node; do + ssh -o BatchMode=yes -o ConnectTimeout=10 "${_node}" \ + 'mkdir -p /tmp/rocm_smi_fix && ln -sf /opt/rocm/lib/librocm_smi64.so.7 /tmp/rocm_smi_fix/librocm_smi64.so.1' \ + 2>/dev/null || echo "[WARN] could not seed rocm_smi fix on ${_node}" >&2 + done < <(scontrol show hostnames "${SLURM_NODELIST}") +fi + +echo "========================================================================" +echo "RCCL A/B regression CI" +echo " Job ID : ${SLURM_JOB_ID:-N/A}" +echo " Nodes : ${SLURM_NNODES:-N/A} (${SLURM_NODELIST:-N/A})" +echo " Config : ${CONFIG_JSON}" +echo " Run log dir : ${RUN_LOG_DIR}" +echo "========================================================================" + +# Per-PR builds of reference + candidate librccl.so now happen in a separate, +# earlier sbatch (sbatch/rccl_build.sbatch, single build-node allocation) so the +# compile doesn't hold this 4-node detection allocation idle. CONFIG_JSON is +# expected to already point at the correct reference/candidate libs by the time +# this script runs — see sbatch/run_rccl_build.sh. + +# Warm up PRTE daemon connections on all nodes before the test loop. +# First mpirun in a fresh allocation races the daemon on one node (cold-start); +# this throwaway hostname ping stabilises all connections before real sweeps begin. +if [[ -n "${SLURM_NODELIST:-}" ]]; then + echo "[INFO] Warming up PRTE daemon connections across all nodes..." + _hostspec=$(scontrol show hostnames "${SLURM_NODELIST}" | awk '{print $1":8"}' | paste -sd,) + _np=$(( $(scontrol show hostnames "${SLURM_NODELIST}" | wc -l) * 8 )) + timeout 60s /it-share/ompi-5.0.8/bin/mpirun \ + --allow-run-as-root \ + -np "${_np}" -H "${_hostspec}" \ + --bind-to numa \ + --mca pml ob1 --mca btl tcp,self \ + --mca oob_tcp_if_include eno0,eno1 \ + --mca btl_tcp_if_include eno0,eno1 \ + hostname >/dev/null 2>&1 \ + && echo "[INFO] PRTE warmup complete." \ + || echo "[WARN] PRTE warmup timed out or failed — continuing anyway." +fi + +cd "${CVS_SBATCH_DIR}" || { echo "[ERROR] cannot cd ${CVS_SBATCH_DIR}" >&2; exit 1; } + +if [[ ! -x run.sh ]]; then chmod +x run.sh 2>/dev/null || true; fi + +./run.sh +pytest_exit=$? + +if [[ -d "${RCCL_CI_ROOT}/ab_artifacts" ]]; then + cp -a "${RCCL_CI_ROOT}/ab_artifacts" "${RUN_LOG_DIR}/" +fi + +if [[ -f "${SLURM_LOG_DIR}/sp_tests-${JOB_TAG}.out" ]]; then + cp -f "${SLURM_LOG_DIR}/sp_tests-${JOB_TAG}.out" "${RUN_LOG_DIR}/slurm.out" 2>/dev/null || true + cp -f "${SLURM_LOG_DIR}/sp_tests-${JOB_TAG}.err" "${RUN_LOG_DIR}/slurm.err" 2>/dev/null || true +fi + +ln -sfn "${RUN_LOG_DIR}" "${RCCL_CI_ROOT}/logs/latest" + +# Gate on confirmed regressions, not on pytest exit code. +# Pytest exits 1 on intermittent harness failures (empty output, SSH blips) even +# when the regression detector found 0 confirmed regressions — those are cluster +# noise, not actual regressions. Re-read the report and derive the gate exit code +# from the confirmed-regression count so the SLURM job only fails on real issues. +_REPORT="${RCCL_CI_ROOT}/ab_artifacts/ab_regression_report.json" +if [[ -f "${_REPORT}" ]]; then + _confirmed=$(python3 -c " +import json, sys +try: + d = json.load(open('${_REPORT}')) + n = sum(r.get('summary', {}).get('regressions', 0) for r in d.get('reports', {}).values()) + print(n) +except Exception as e: + print('ERR', file=sys.stderr); sys.exit(2) +" 2>/dev/null) + if [[ "${_confirmed}" == "0" ]]; then + echo "[INFO] Verdict: PASS — 0 confirmed regressions (pytest_exit=${pytest_exit})" + exit_code=0 + elif [[ "${_confirmed}" =~ ^[0-9]+$ ]]; then + echo "[INFO] Verdict: FAIL — ${_confirmed} confirmed regression(s)" + exit_code=1 + else + echo "[WARN] Could not parse report; falling back to pytest exit code ${pytest_exit}" + exit_code="${pytest_exit}" + fi +else + echo "[WARN] Report not found at ${_REPORT}; using pytest exit code ${pytest_exit}" + exit_code="${pytest_exit}" +fi + +echo "[INFO] A/B run finished with exit code ${exit_code} (pytest_exit=${pytest_exit})" +echo "[INFO] Artifacts: ${RUN_LOG_DIR}" +exit "${exit_code}" diff --git a/ci/rccl_perf_gate/sbatch/run_rccl_build.sh b/ci/rccl_perf_gate/sbatch/run_rccl_build.sh new file mode 100755 index 000000000..7fff992e3 --- /dev/null +++ b/ci/rccl_perf_gate/sbatch/run_rccl_build.sh @@ -0,0 +1,33 @@ +#!/usr/bin/env bash +# RCCL build-only runner: build reference + candidate librccl.so and template +# their lib paths into CONFIG_JSON. No MPI/detect happens here — see +# run_rccl_ab.sh for the 4-node detection step that follows. +# +# Invoked by sbatch/rccl_build.sbatch. Env contract: +# CANDIDATE_SRC rocm-systems checkout at PR head +# BASE_REF ref to merge-base against for the reference build +# CONFIG_JSON ci_detect.json to template (defaults to configs/ci_detect.json) + +set -euo pipefail + +readonly RCCL_CI_ROOT="${RCCL_CI_ROOT:-/it-share/rccl-ci}" + +: "${CANDIDATE_SRC:?run_rccl_build.sh requires CANDIDATE_SRC (RCCL PR checkout)}" +: "${BASE_REF:?run_rccl_build.sh requires BASE_REF (e.g. origin/develop)}" +export CONFIG_JSON="${CONFIG_JSON:-${RCCL_CI_ROOT}/configs/ci_detect.json}" + +[[ -f "${CONFIG_JSON}" ]] || { echo "[ERROR] CONFIG_JSON not found: ${CONFIG_JSON}" >&2; exit 1; } + +mkdir -p "${RCCL_CI_ROOT}/logs" + +echo "========================================================================" +echo "RCCL build-only CI" +echo " Job ID : ${SLURM_JOB_ID:-N/A}" +echo " Node : ${SLURMD_NODENAME:-N/A}" +echo " Config : ${CONFIG_JSON}" +echo "========================================================================" + +build_args=(--candidate-src "${CANDIDATE_SRC}" --base-ref "${BASE_REF}" --config "${CONFIG_JSON}") +[[ -n "${BUILD_OUT:-}" ]] && build_args+=(--out "${BUILD_OUT}") + +bash "${RCCL_CI_ROOT}/cvs-sbatch/lib/build_rccl.sh" "${build_args[@]}" diff --git a/ci/rccl_perf_gate/submit_and_poll.sh b/ci/rccl_perf_gate/submit_and_poll.sh new file mode 100755 index 000000000..4b821ab03 --- /dev/null +++ b/ci/rccl_perf_gate/submit_and_poll.sh @@ -0,0 +1,94 @@ +#!/usr/bin/env bash +############################################################## +# Submit the RCCL A/B regression sbatch job, wait for it to finish, and map the +# job's exit code to this script's exit code so a CI step can gate on it. +# +# Usage: +# cvs/ci/rccl_perf_gate/submit_and_poll.sh [CONFIG_JSON] +# +# Env overrides: +# CONFIG_JSON benchmark config (default: configs/ci_detect.json) +# SBATCH_SCRIPT sbatch entry point (default: cvs/ci/rccl_perf_gate/sbatch/rccl_ab.sbatch) +# POLL_INTERVAL seconds between status polls (default: 30) +# MAX_WAIT_SEC hard timeout in seconds (default: 28800 = 8h) +# NODELIST override pinned nodes (passed to sbatch --nodelist) +# +# Exit codes: +# 0 job completed, gate PASS (pytest exit 0) +# 1 job completed, regression detected / pytest failure +# 2 submission or polling error (job never produced a terminal state) +############################################################## +set -euo pipefail + +readonly RCCL_CI_ROOT="${RCCL_CI_ROOT:-/it-share/rccl-ci}" +readonly CONFIG_JSON="${1:-${CONFIG_JSON:-${RCCL_CI_ROOT}/configs/ci_detect.json}}" +readonly SBATCH_SCRIPT="${SBATCH_SCRIPT:-${RCCL_CI_ROOT}/cvs/ci/rccl_perf_gate/sbatch/rccl_ab.sbatch}" +readonly POLL_INTERVAL="${POLL_INTERVAL:-30}" +readonly MAX_WAIT_SEC="${MAX_WAIT_SEC:-28800}" + +log() { echo "[$(date +%H:%M:%S)] $*"; } + +[[ -f "${CONFIG_JSON}" ]] || { echo "[ERROR] config not found: ${CONFIG_JSON}" >&2; exit 2; } +[[ -f "${SBATCH_SCRIPT}" ]] || { echo "[ERROR] sbatch script not found: ${SBATCH_SCRIPT}" >&2; exit 2; } + +# Forward the optional Phase-4 build vars so the detector job builds ref+cand +# librccl in-allocation before detecting (CI detect mode sets BUILD_RCCL=1). +export_list="ALL,CONFIG_JSON=${CONFIG_JSON}" +if [[ "${BUILD_RCCL:-0}" == "1" ]]; then + : "${CANDIDATE_SRC:?BUILD_RCCL=1 requires CANDIDATE_SRC}" + : "${BASE_REF:?BUILD_RCCL=1 requires BASE_REF}" + export_list+=",BUILD_RCCL=1,CANDIDATE_SRC=${CANDIDATE_SRC},BASE_REF=${BASE_REF}" +fi + +submit_args=(--parsable --export="${export_list}") +[[ -n "${NODELIST:-}" ]] && submit_args+=(--nodelist="${NODELIST}") +# Pass reservation only if set — the runner user may not have access to it +[[ -n "${SLURM_RESERVATION:-}" ]] && submit_args+=(--reservation="${SLURM_RESERVATION}") + +log "Submitting ${SBATCH_SCRIPT} (config=${CONFIG_JSON})" +JOB_ID="$(sbatch "${submit_args[@]}" "${SBATCH_SCRIPT}")" +JOB_ID="${JOB_ID%%;*}" # strip cluster suffix from --parsable output +[[ -n "${JOB_ID}" ]] || { echo "[ERROR] sbatch returned no job id" >&2; exit 2; } +log "Submitted job ${JOB_ID}" + +# Poll until the job leaves the queue / reaches a terminal state. +elapsed=0 +state="" +while (( elapsed < MAX_WAIT_SEC )); do + # squeue lists only pending/running jobs; empty => terminal. + if ! squeue -h -j "${JOB_ID}" -o "%T" 2>/dev/null | grep -q .; then + break + fi + state="$(squeue -h -j "${JOB_ID}" -o "%T" 2>/dev/null | head -1)" + log "job ${JOB_ID} state=${state} (${elapsed}s)" + sleep "${POLL_INTERVAL}" + elapsed=$(( elapsed + POLL_INTERVAL )) +done + +if (( elapsed >= MAX_WAIT_SEC )); then + log "[ERROR] timeout after ${MAX_WAIT_SEC}s; cancelling job ${JOB_ID}" + scancel "${JOB_ID}" 2>/dev/null || true + exit 2 +fi + +# Resolve the final exit code from accounting. Retry: sacct can lag briefly. +final_state="" +exit_code="" +for _ in $(seq 1 10); do + read -r final_state exit_code < <( + sacct -n -X -j "${JOB_ID}" -o State,ExitCode 2>/dev/null \ + | head -1 | awk '{print $1, $2}' + ) || true + [[ -n "${final_state}" ]] && break + sleep 5 +done + +log "job ${JOB_ID} final_state=${final_state:-unknown} exit=${exit_code:-unknown}" + +# ExitCode is "code:signal"; take the code. +rc="${exit_code%%:*}" +case "${final_state}" in + COMPLETED) exit "${rc:-0}" ;; + FAILED) exit "${rc:-1}" ;; + *) log "[ERROR] job ended in non-success state ${final_state}"; exit 2 ;; +esac From bcf2770222af574e37bd55392e3e2af764b48b2f Mon Sep 17 00:00:00 2001 From: speriasw Date: Tue, 11 Aug 2026 12:54:10 +0000 Subject: [PATCH 06/17] ci: per-run workspace isolation + ROCm dist layout invariant Workspace isolation (RCCL_CI_WORKSPACE=1, default off): run build and detect out of runs// with their own cvs worktree, cvs-sbatch copy, builds, artifacts and logs, so concurrent runs cannot clobber each other. Prerequisite for adding more GitHub runners. Legacy shared-tree behaviour is byte-identical when the flag is off. Builds are seeded from a shared rev+recipe-keyed cache via hardlinks so isolation does not cost a cold ~30min LTO build per run. ROCm dist layout invariant: fail the build fast if the SDK has several inodes per SONAME. The dist is a pip wheel and wheel packaging flattens versioned-library symlink chains into independent files; glibc dedups loaded objects by inode, so a dlopen() by unversioned name returns a second, uninitialised copy. For ROCr that silently disabled DMA-BUF and hung multi-node collectives for the full rccl_timeout. --- ci/rccl_perf_gate/sbatch/run_rccl_ab.sh | 56 +++++++++++-- ci/rccl_perf_gate/sbatch/run_rccl_build.sh | 95 ++++++++++++++++++++++ 2 files changed, 144 insertions(+), 7 deletions(-) diff --git a/ci/rccl_perf_gate/sbatch/run_rccl_ab.sh b/ci/rccl_perf_gate/sbatch/run_rccl_ab.sh index 63e30fcd9..7df201241 100755 --- a/ci/rccl_perf_gate/sbatch/run_rccl_ab.sh +++ b/ci/rccl_perf_gate/sbatch/run_rccl_ab.sh @@ -1,6 +1,11 @@ #!/usr/bin/env bash # Shared RCCL A/B regression runner (no #SBATCH directives). # Invoked by sbatch/rccl_ab.sbatch or manually via srun inside an allocation. +# +# Optional (per-run workspace isolation — see sbatch/lib/workspace.sh): +# RCCL_CI_WORKSPACE=1 run out of runs// (own cvs worktree, own +# cvs-sbatch copy, own artifacts and logs) instead of the +# shared trees. Default 0 = legacy shared behaviour. set -euo pipefail @@ -20,14 +25,36 @@ if [[ -n "${SLURM_JOB_ID:-}" && -n "${SLURM_STEP_ID:-}" ]]; then exit 1 fi -export RUN_LOG_DIR="${RCCL_CI_ROOT}/logs/run_${TIMESTAMP}_${JOB_TAG}" -mkdir -p "${RUN_LOG_DIR}" +# --- per-run workspace (no-op unless RCCL_CI_WORKSPACE=1) --------------------- +# shellcheck source=/dev/null +source "${RCCL_CI_ROOT}/sbatch/lib/workspace.sh" + +# ws_init is idempotent: the build job created this workspace, we re-enter it. +# It is also safe if detect runs without a preceding build (manual submission) — +# the workspace is created fresh and the config's lib paths are whatever the +# caller set. +if ws_enabled; then + ws_init || { echo "[ERROR] workspace init failed" >&2; exit 1; } + ws_begin + trap 'ws_end' EXIT +fi + +if ws_enabled; then + export RUN_LOG_DIR="$(ws_root)/logs" + export CVS_DIR="$(ws_root)/cvs" + export CVS_SBATCH_DIR="$(ws_root)/cvs-sbatch" + AB_ARTIFACT_DIR="$(ws_root)/artifacts" +else + export RUN_LOG_DIR="${RCCL_CI_ROOT}/logs/run_${TIMESTAMP}_${JOB_TAG}" + export CVS_DIR="${RCCL_CI_ROOT}/cvs" + export CVS_SBATCH_DIR="${RCCL_CI_ROOT}/cvs-sbatch" + AB_ARTIFACT_DIR="${RCCL_CI_ROOT}/ab_artifacts" +fi +mkdir -p "${RUN_LOG_DIR}" "${AB_ARTIFACT_DIR}" readonly SLURM_LOG_DIR="${RCCL_CI_ROOT}/logs" mkdir -p "${SLURM_LOG_DIR}" -export CVS_DIR="${RCCL_CI_ROOT}/cvs" -export CVS_SBATCH_DIR="${RCCL_CI_ROOT}/cvs-sbatch" export SKIP_CVS_SETUP=1 export CONFIG_JSON="${CONFIG_JSON:-${RCCL_CI_ROOT}/configs/ab_robustness.json}" # Normalise to an absolute path. run.sh runs from cvs-sbatch/ and resolves the @@ -42,6 +69,11 @@ if [[ "${CONFIG_JSON}" != /* ]]; then export CONFIG_JSON fi [[ -f "${CONFIG_JSON}" ]] || { echo "[ERROR] CONFIG_JSON not found: ${CONFIG_JSON}" >&2; exit 1; } + +# Point output_dir and the /tmp sweep scratch at this run. Idempotent — the +# build job already did this, but detect may run standalone. +ws_config_retarget "${CONFIG_JSON}" || true + export TEST_PATH="${TEST_PATH:-./cvs/tests/rccl/rccl_ab_regression.py}" export LOG_FILE="${RUN_LOG_DIR}/pytest.log" @@ -61,6 +93,11 @@ echo " Job ID : ${SLURM_JOB_ID:-N/A}" echo " Nodes : ${SLURM_NNODES:-N/A} (${SLURM_NODELIST:-N/A})" echo " Config : ${CONFIG_JSON}" echo " Run log dir : ${RUN_LOG_DIR}" +if ws_enabled; then +echo " Workspace : $(ws_root)" +echo " Artifacts : ${AB_ARTIFACT_DIR}" +echo " CVS worktree : ${CVS_DIR}" +fi echo "========================================================================" # Per-PR builds of reference + candidate librccl.so now happen in a separate, @@ -95,8 +132,11 @@ if [[ ! -x run.sh ]]; then chmod +x run.sh 2>/dev/null || true; fi ./run.sh pytest_exit=$? -if [[ -d "${RCCL_CI_ROOT}/ab_artifacts" ]]; then - cp -a "${RCCL_CI_ROOT}/ab_artifacts" "${RUN_LOG_DIR}/" +# In workspace mode artifacts already live inside the workspace next to logs/, +# so there is nothing to copy. In legacy mode snapshot the shared dir into the +# per-run log dir before the next run overwrites it. +if ! ws_enabled && [[ -d "${AB_ARTIFACT_DIR}" ]]; then + cp -a "${AB_ARTIFACT_DIR}" "${RUN_LOG_DIR}/" fi if [[ -f "${SLURM_LOG_DIR}/sp_tests-${JOB_TAG}.out" ]]; then @@ -111,7 +151,7 @@ ln -sfn "${RUN_LOG_DIR}" "${RCCL_CI_ROOT}/logs/latest" # when the regression detector found 0 confirmed regressions — those are cluster # noise, not actual regressions. Re-read the report and derive the gate exit code # from the confirmed-regression count so the SLURM job only fails on real issues. -_REPORT="${RCCL_CI_ROOT}/ab_artifacts/ab_regression_report.json" +_REPORT="${AB_ARTIFACT_DIR}/ab_regression_report.json" if [[ -f "${_REPORT}" ]]; then _confirmed=$(python3 -c " import json, sys @@ -137,6 +177,8 @@ else exit_code="${pytest_exit}" fi +ws_record verdict_exit_code "${exit_code}" || true + echo "[INFO] A/B run finished with exit code ${exit_code} (pytest_exit=${pytest_exit})" echo "[INFO] Artifacts: ${RUN_LOG_DIR}" exit "${exit_code}" diff --git a/ci/rccl_perf_gate/sbatch/run_rccl_build.sh b/ci/rccl_perf_gate/sbatch/run_rccl_build.sh index 7fff992e3..7240216a9 100755 --- a/ci/rccl_perf_gate/sbatch/run_rccl_build.sh +++ b/ci/rccl_perf_gate/sbatch/run_rccl_build.sh @@ -7,6 +7,13 @@ # CANDIDATE_SRC rocm-systems checkout at PR head # BASE_REF ref to merge-base against for the reference build # CONFIG_JSON ci_detect.json to template (defaults to configs/ci_detect.json) +# +# Optional (per-run workspace isolation — see sbatch/lib/workspace.sh): +# RCCL_CI_WORKSPACE=1 build into runs//builds instead of the shared +# builds/ dir, so concurrent runs cannot clobber each +# other's librccl. Default 0 = legacy shared behaviour. +# RCCL_CI_BUILD_CACHE=0 disable the rev-keyed shared lib cache (forces a cold +# build every run). Default 1. set -euo pipefail @@ -18,16 +25,104 @@ export CONFIG_JSON="${CONFIG_JSON:-${RCCL_CI_ROOT}/configs/ci_detect.json}" [[ -f "${CONFIG_JSON}" ]] || { echo "[ERROR] CONFIG_JSON not found: ${CONFIG_JSON}" >&2; exit 1; } +# --- ROCm dist layout invariant ---------------------------------------------- +# The SDK is a pip wheel, and wheel packaging flattens versioned-library symlink +# chains into independent regular files. When that happens, one SONAME maps to +# several inodes; glibc dedups loaded objects by inode, so a dlopen() by +# unversioned name yields a SECOND, uninitialised copy of the library. For ROCr +# that silently disables DMA-BUF export and multi-node collectives hang for the +# full rccl_timeout instead of failing. +# +# This is a ~1s read-only check. It runs before the ~30min build so a re-flattened +# dist (any SDK reinstall reintroduces it) fails here with a named cause, rather +# than surfacing hours later as an unexplained timeout. +_norm="${RCCL_CI_ROOT}/sbatch/lib/normalize_rocm_dist.sh" +if [[ -x "${_norm}" ]]; then + if ! _norm_out="$("${_norm}" --check 2>&1)"; then + echo "${_norm_out}" >&2 + echo "[ERROR] ROCm dist layout invariant violated — refusing to build." >&2 + exit 1 + fi + echo "[INFO] ROCm dist layout OK (one inode per SONAME)." +else + echo "[WARN] ${_norm} not found; skipping ROCm dist layout check." >&2 +fi + mkdir -p "${RCCL_CI_ROOT}/logs" +# --- per-run workspace (no-op unless RCCL_CI_WORKSPACE=1) --------------------- +# shellcheck source=/dev/null +source "${RCCL_CI_ROOT}/sbatch/lib/workspace.sh" + +if ws_enabled; then + ws_init || { echo "[ERROR] workspace init failed" >&2; exit 1; } + ws_begin + # Release the in-use marker however we exit, so gc can reap this run later. + trap 'ws_end' EXIT + + ws_config_retarget "${CONFIG_JSON}" || true + + # Build into the workspace rather than the shared builds/ dir. build_rccl.sh + # already honours --out; run_rccl_build.sh just points it here. + BUILD_OUT="$(ws_root)/builds" + export BUILD_OUT +fi + echo "========================================================================" echo "RCCL build-only CI" echo " Job ID : ${SLURM_JOB_ID:-N/A}" echo " Node : ${SLURMD_NODENAME:-N/A}" echo " Config : ${CONFIG_JSON}" +if ws_enabled; then +echo " Workspace : $(ws_root)" +echo " Build out : ${BUILD_OUT}" +echo " Build cache : $(ws_cache_enabled && echo enabled || echo disabled)" +fi echo "========================================================================" +# --- seed both sides from the shared rev-keyed lib cache ---------------------- +# build_rccl.sh skips a side when lib/librccl.so exists and lib/.built_rev +# matches the revision it wants. We compute the same two revisions here and, on +# a cache hit, lay down a complete lib/ (stamp included) before calling it — so +# its own cache logic does the skipping and build_rccl.sh needs no changes. +# +# If our revision computation ever disagrees with build_rccl.sh's, the worst +# case is a cache miss and a rebuild: it re-validates .built_rev itself, so a +# mismatched lib can never be silently accepted. +if ws_enabled && ws_cache_enabled; then + git -C "${CANDIDATE_SRC}" config --global --add safe.directory "${CANDIDATE_SRC}" 2>/dev/null || true + git -C "${CANDIDATE_SRC}" fetch --no-tags origin "${BASE_REF#origin/}" 2>/dev/null || true + + _cand_rev="$(git -C "${CANDIDATE_SRC}" rev-parse HEAD 2>/dev/null || echo unknown)" + _ref_rev="$(git -C "${CANDIDATE_SRC}" merge-base HEAD "${BASE_REF}" 2>/dev/null || echo unknown)" + + echo "[INFO] reference rev=${_ref_rev:0:10} candidate rev=${_cand_rev:0:10}" + ws_record reference_rev "${_ref_rev}" + ws_record candidate_rev "${_cand_rev}" + + ws_cache_fetch reference "${_ref_rev}" || true + ws_cache_fetch candidate "${_cand_rev}" || true +fi + build_args=(--candidate-src "${CANDIDATE_SRC}" --base-ref "${BASE_REF}" --config "${CONFIG_JSON}") [[ -n "${BUILD_OUT:-}" ]] && build_args+=(--out "${BUILD_OUT}") bash "${RCCL_CI_ROOT}/cvs-sbatch/lib/build_rccl.sh" "${build_args[@]}" + +# --- publish to the cache, then drop the object trees ------------------------- +# Order matters: publish before pruning, and prune only after lib/ is safely +# cached. ws_prune_build refuses to prune a side that has no librccl.so, so a +# failed build keeps its full tree for post-mortem. +if ws_enabled; then + ws_cache_publish reference + ws_cache_publish candidate + ws_prune_build reference + ws_prune_build candidate + + echo "[INFO] workspace: $(ws_root)" + du -sh "$(ws_root)" 2>/dev/null || true + + # Opportunistic: reap old workspaces on the build node, where there is no + # 4-node allocation being held hostage by the cleanup. + ws_gc || true +fi From e834c074216219f11470989b482b74026b91291a Mon Sep 17 00:00:00 2001 From: speriasw Date: Tue, 11 Aug 2026 13:19:28 +0000 Subject: [PATCH 07/17] ci: never fail a successful build on post-build housekeeping ws_prune_build ran du over build/, stage/ and src/ unconditionally. stage/ is not always created and the candidate side builds in place so it has no src/; du then returns non-zero, pipefail propagates that out of the assignment, and set -e aborted a build whose libraries were already built, cached and templated. Via the afterok dependency that also silently cancelled the detect job behind it. Measure only paths that exist, and treat cache-publish and prune as non-fatal at the call sites. --- ci/rccl_perf_gate/sbatch/run_rccl_build.sh | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/ci/rccl_perf_gate/sbatch/run_rccl_build.sh b/ci/rccl_perf_gate/sbatch/run_rccl_build.sh index 7240216a9..0e4213a23 100755 --- a/ci/rccl_perf_gate/sbatch/run_rccl_build.sh +++ b/ci/rccl_perf_gate/sbatch/run_rccl_build.sh @@ -114,10 +114,14 @@ bash "${RCCL_CI_ROOT}/cvs-sbatch/lib/build_rccl.sh" "${build_args[@]}" # cached. ws_prune_build refuses to prune a side that has no librccl.so, so a # failed build keeps its full tree for post-mortem. if ws_enabled; then - ws_cache_publish reference - ws_cache_publish candidate - ws_prune_build reference - ws_prune_build candidate + # Housekeeping must never fail a build that already produced good libraries. + # Caching and pruning are optimisations: losing either costs disk or a rebuild, + # whereas aborting here throws away a successful ~10min compile and, via the + # afterok dependency, silently cancels the detect job behind it. + ws_cache_publish reference || ws_warn "cache publish (reference) failed — non-fatal" + ws_cache_publish candidate || ws_warn "cache publish (candidate) failed — non-fatal" + ws_prune_build reference || ws_warn "prune (reference) failed — non-fatal" + ws_prune_build candidate || ws_warn "prune (candidate) failed — non-fatal" echo "[INFO] workspace: $(ws_root)" du -sh "$(ws_root)" 2>/dev/null || true From 29feb420bbbb164d4d98e9adb934098a8827530f Mon Sep 17 00:00:00 2001 From: surya periaswamy Date: Tue, 11 Aug 2026 14:50:38 +0000 Subject: [PATCH 08/17] ci: version the perf-gate helper libs alongside the scripts that need them run_rccl_build.sh and run_rccl_ab.sh source workspace.sh and exec normalize_rocm_dist.sh from ${RCCL_CI_ROOT}/sbatch/lib -- an unversioned NFS directory. So the committed scripts depended on code that was not in the repo: a fresh checkout of ROCm/cvs would hard-fail on the `source`, and normalize_rocm_dist.sh would be silently skipped, which reintroduces the flattened-dist DMA-BUF hang the invariant check exists to catch. Bring all three into the tree next to their callers. The callers now resolve lib/ relative to their own location and only fall back to the NFS path, so both the cvs checkout and the hand-maintained /it-share/rccl-ci/sbatch copies keep working. check_dmabuf.sh is new: it probes both A/B sides for ROCr resolution and DMA-BUF support. Beyond liveness it enforces symmetry, because a pair where reference has DMA-BUF off and candidate has it on does not merely run slowly -- the sides use different transports, so the candidate scores a large fake improvement and the gate reports a false negative. Co-Authored-By: Claude Opus 5 --- ci/rccl_perf_gate/sbatch/lib/check_dmabuf.sh | 189 ++++++ .../sbatch/lib/normalize_rocm_dist.sh | 228 ++++++++ ci/rccl_perf_gate/sbatch/lib/workspace.sh | 550 ++++++++++++++++++ 3 files changed, 967 insertions(+) create mode 100755 ci/rccl_perf_gate/sbatch/lib/check_dmabuf.sh create mode 100755 ci/rccl_perf_gate/sbatch/lib/normalize_rocm_dist.sh create mode 100755 ci/rccl_perf_gate/sbatch/lib/workspace.sh diff --git a/ci/rccl_perf_gate/sbatch/lib/check_dmabuf.sh b/ci/rccl_perf_gate/sbatch/lib/check_dmabuf.sh new file mode 100755 index 000000000..fb0d7a36c --- /dev/null +++ b/ci/rccl_perf_gate/sbatch/lib/check_dmabuf.sh @@ -0,0 +1,189 @@ +#!/usr/bin/env bash +# Assert that RCCL resolves ROCr correctly and that BOTH A/B sides agree on the +# transport capabilities they will use. +# +# WHY +# --- +# RCCL probes ROCr at init via dlopen("libhsa-runtime64.so"). If the ROCm dist +# has a flattened layout (several inodes for one SONAME — see +# normalize_rocm_dist.sh), that dlopen returns a SECOND, uninitialised copy of +# ROCr. hsa_system_get_info then returns 4107, RCCL jumps to its error path, +# pfn_hsa_amd_portable_export_dmabuf is never resolved, and DMA-BUF export is +# silently disabled. Multi-node collectives hang for the full rccl_timeout. +# +# Whether a given librccl is affected depends on how it was BUILT: a build that +# links ROCr via DT_NEEDED is immune, one that relies on dlopen is not. That is +# the dangerous part for an A/B gate — reference and candidate can differ. When +# reference has DMA-BUF off and candidate has it on, the comparison is not +# merely slow, it is INVALID: the candidate scores as a huge fake improvement. +# +# So this script checks two things: +# 1. liveness — each side actually has DMA-BUF enabled +# 2. symmetry — both sides resolved the SAME capabilities, so the A/B +# measurement is comparing library changes and nothing else +# +# USAGE +# check_dmabuf.sh --lib DIR # probe one side by lib directory +# check_dmabuf.sh --ldpath 'A:B:C' # probe one side by full LD_LIBRARY_PATH +# check_dmabuf.sh --config ci_detect.json # probe reference AND candidate, compare +# +# Requires GPUs: run inside an allocation with a GRES request (e.g. +# --exclusive --gres=gpu:8). Without a gres request Slurm's device cgroup hides +# /dev/kfd and the probe reports "no ROCm-capable device is detected". +# +# Exit 0 = healthy (and symmetric, in --config mode) +# 1 = DMA-BUF disabled on a side, or the two sides disagree +# 2 = inconclusive / harness failure + +set -uo pipefail + +RCCL_CI_ROOT="${RCCL_CI_ROOT:-/it-share/rccl-ci}" +ROCM_DIST="${ROCM_DIST:-${RCCL_CI_ROOT}/rocm_devel}" +PERF_BIN="${RCCL_CI_ROOT}/rccl-tests-2.30.4/bin/all_reduce_perf" +NGPU="${DMABUF_CHECK_NGPU:-2}" + +LIB_DIR=""; LD_PATH=""; CONFIG=""; ROCR_PATH="" + +while [[ $# -gt 0 ]]; do + case "$1" in + --lib) LIB_DIR="$2"; shift 2 ;; + --ldpath) LD_PATH="$2"; shift 2 ;; + --config) CONFIG="$2"; shift 2 ;; + --rocr-path) ROCR_PATH="$2"; shift 2 ;; + --ngpu) NGPU="$2"; shift 2 ;; + -h|--help) sed -n '2,40p' "$0"; exit 0 ;; + *) echo "[ERROR] unknown arg: $1" >&2; exit 2 ;; + esac +done + +[[ -x "${PERF_BIN}" ]] || { echo "[ERROR] missing ${PERF_BIN}" >&2; exit 2; } + +# --- probe one side ----------------------------------------------------------- +# Echoes " " on stdout; +# human-readable detail goes to stderr so callers can capture the verdict alone. +# dmabuf is one of: enabled | disabled | unknown +probe_side() { + local label="$1" ldpath="$2" + local log resolved hsa_needed dmabuf rocr rc + + log="$(mktemp)" + + resolved="$(LD_LIBRARY_PATH="${ldpath}" ldd "${PERF_BIN}" 2>/dev/null | awk '/librccl/ {print $3}')" + hsa_needed="$(readelf -d "${resolved}" 2>/dev/null | grep -c 'hsa-runtime')" + [[ -z "${hsa_needed}" ]] && hsa_needed=0 + + { + echo "--- ${label} ---" + echo " LD_LIBRARY_PATH : ${ldpath}" + echo " resolved librccl: ${resolved:-}" + echo " hsa DT_NEEDED : ${hsa_needed} (0 = dlopen path, sensitive to dist layout)" + } >&2 + + env -i \ + PATH="/usr/bin:/bin" \ + HOME="${HOME}" \ + LD_LIBRARY_PATH="${ldpath}" \ + NCCL_DEBUG=INFO \ + NCCL_DEBUG_SUBSYS=INIT \ + HSA_NO_SCRATCH_RECLAIM=1 \ + NCCL_IGNORE_CPU_AFFINITY=1 \ + ${ROCR_PATH:+RCCL_ROCR_PATH="${ROCR_PATH}"} \ + timeout 180s "${PERF_BIN}" -b 8 -e 8 -f 2 -g "${NGPU}" > "${log}" 2>&1 + rc=$? + + rocr="$(grep -oiE 'ROCr version [0-9.]+' "${log}" | head -1 | awk '{print $3}')" + [[ -z "${rocr}" ]] && rocr="none" + + if grep -qi 'DMA_BUF Support Enabled' "${log}"; then + dmabuf="enabled" + elif grep -qE '4107|DMA_BUF Support Disabled|Could not find .*dmabuf' "${log}"; then + dmabuf="disabled" + else + dmabuf="unknown" + { echo " [!] no ROCr verdict (perf exit=${rc}); tail:"; tail -12 "${log}" | sed 's/^/ /'; } >&2 + fi + + grep -iE 'rocr|dma.?buf|4107' "${log}" | sed 's/^/ /' >&2 + echo " => dmabuf=${dmabuf} rocr=${rocr}" >&2 + echo >&2 + + rm -f "${log}" + echo "${dmabuf} ${rocr} ${hsa_needed} ${resolved:-none}" +} + +echo "==========================================================================" +echo "RCCL transport capability preflight" +echo " node : $(hostname)" +echo " rocm dist : ${ROCM_DIST}" +echo " rocr path : ${ROCR_PATH:-}" +echo "==========================================================================" + +# --- single-side mode --------------------------------------------------------- +if [[ -z "${CONFIG}" ]]; then + if [[ -n "${LIB_DIR}" ]]; then + LD_PATH="${LIB_DIR}:${ROCM_DIST}/lib:/it-share/ompi-5.0.8/lib" + fi + [[ -n "${LD_PATH}" ]] || { echo "[ERROR] need --lib, --ldpath or --config" >&2; exit 2; } + + read -r dmabuf rocr _hsa _lib <<< "$(probe_side "side" "${LD_PATH}")" + case "${dmabuf}" in + enabled) echo "[PASS] DMA-BUF enabled (ROCr ${rocr})."; exit 0 ;; + disabled) echo "[FAIL] DMA-BUF DISABLED — RCCL loaded a second, uninitialised ROCr." >&2 + echo " Check dist layout: ${RCCL_CI_ROOT}/sbatch/lib/normalize_rocm_dist.sh --check" >&2 + exit 1 ;; + *) echo "[INCONCLUSIVE] no ROCr verdict." >&2; exit 2 ;; + esac +fi + +# --- A/B mode: probe both sides and compare ----------------------------------- +[[ -f "${CONFIG}" ]] || { echo "[ERROR] config not found: ${CONFIG}" >&2; exit 2; } + +read -r REF_LD CAND_LD <<< "$(python3 -c " +import json,sys +d=json.load(open('${CONFIG}')) +ab=d.get('rccl',{}).get('ab_regression',{}) +r=ab.get('reference',{}).get('ld_library_path','') +c=ab.get('candidate',{}).get('ld_library_path','') +if not r or not c: sys.exit(3) +print(r,c) +" 2>/dev/null)" || { echo "[ERROR] could not read ld_library_path for both sides from ${CONFIG}" >&2; exit 2; } + +read -r R_DMABUF R_ROCR R_HSA R_LIB <<< "$(probe_side "reference" "${REF_LD}")" +read -r C_DMABUF C_ROCR C_HSA C_LIB <<< "$(probe_side "candidate" "${CAND_LD}")" + +echo "=== capability summary ===" +printf ' %-10s dmabuf=%-9s rocr=%-6s hsa_needed=%s\n' "reference" "${R_DMABUF}" "${R_ROCR}" "${R_HSA}" +printf ' %-10s dmabuf=%-9s rocr=%-6s hsa_needed=%s\n' "candidate" "${C_DMABUF}" "${C_ROCR}" "${C_HSA}" +echo + +status=0 + +# 1. Liveness. A side without DMA-BUF will hang the multi-node collectives. +for side in reference candidate; do + v="R_DMABUF"; [[ "${side}" == "candidate" ]] && v="C_DMABUF" + case "${!v}" in + enabled) ;; + disabled) + echo "[FAIL] ${side}: DMA-BUF is DISABLED — multi-node collectives will hang." >&2 + echo " Fix the dist layout: ${RCCL_CI_ROOT}/sbatch/lib/normalize_rocm_dist.sh --check" >&2 + status=1 ;; + *) + echo "[WARN] ${side}: capability probe inconclusive." >&2 + [[ ${status} -eq 0 ]] && status=2 ;; + esac +done + +# 2. Symmetry. This is the correctness gate: differing capabilities mean the A/B +# result measures the environment, not the code change under test. +if [[ "${R_DMABUF}" != "${C_DMABUF}" ]]; then + echo "[FAIL] A/B ASYMMETRY: reference dmabuf=${R_DMABUF} but candidate dmabuf=${C_DMABUF}." >&2 + echo " The two sides would not use the same transport path, so any" >&2 + echo " measured delta reflects the environment, not the code change." >&2 + echo " Refusing to report a verdict from an invalid comparison." >&2 + status=1 +fi + +if [[ ${status} -eq 0 ]]; then + echo "[PASS] both sides: DMA-BUF enabled, ROCr ${R_ROCR} — capabilities symmetric." +fi +exit "${status}" diff --git a/ci/rccl_perf_gate/sbatch/lib/normalize_rocm_dist.sh b/ci/rccl_perf_gate/sbatch/lib/normalize_rocm_dist.sh new file mode 100755 index 000000000..cbafce04d --- /dev/null +++ b/ci/rccl_perf_gate/sbatch/lib/normalize_rocm_dist.sh @@ -0,0 +1,228 @@ +#!/usr/bin/env bash +# Normalise a ROCm SDK lib/ directory so each SONAME resolves to exactly ONE inode. +# +# WHY THIS EXISTS +# --------------- +# rocm_devel is a pip-installed wheel (rocm_sdk_devel-*.whl). Wheels are zip +# archives, and the packaging step flattened most versioned-library symlink +# chains into independent regular files. The wheel RECORD confirms it: +# +# _rocm_sdk_devel/lib/libhsa-runtime64.so,, +# _rocm_sdk_devel/lib/libhsa-runtime64.so.1,, +# _rocm_sdk_devel/lib/libhsa-runtime64.so.1.21.0,, +# +# Three separate entries -> three regular files -> three DIFFERENT INODES with +# byte-identical content and the same SONAME. +# +# That breaks a load-bearing glibc invariant: the dynamic loader dedups already +# loaded shared objects by (device, inode), NOT by filename or SONAME. So a +# process that links libhsa-runtime64.so.1 via DT_NEEDED and later +# dlopen()s "libhsa-runtime64.so" gets a SECOND, INDEPENDENT, NEVER-INITIALISED +# copy of ROCr. Every hsa_* call on that handle returns 4107 +# (HSA_STATUS_ERROR_NOT_INITIALIZED). +# +# In RCCL that path is rocmwrap.cc: the dlopen fails its version probe, jumps to +# `error:`, and pfn_hsa_amd_portable_export_dmabuf is never resolved -- DMA-BUF +# is silently disabled, with no message unless NCCL_DEBUG>=WARN. Multi-node +# collectives then fall back to a path that hangs on this fabric. +# +# Reinstalling or rebuilding the SDK does NOT fix this: the same wheel produces +# the same flattened layout. The fix belongs here, as a post-install step that +# runs every time the dist is refreshed. +# +# WHAT IT DOES +# ------------ +# For each group of byte-identical regular files whose names form a versioned +# chain (libfoo.so, libfoo.so.1, libfoo.so.1.2.3), keep the most-versioned file +# as the single real object and replace the shorter names with relative symlinks +# forming the conventional chain: +# +# libfoo.so -> libfoo.so.1 -> libfoo.so.1.2.3 (one inode) +# +# This is exactly the layout the ROCm .deb/.tar ships and what ldconfig would +# produce. It is content-preserving: no bytes change, so processes holding the +# old inodes open are unaffected. +# +# USAGE +# ----- +# normalize_rocm_dist.sh # dry run against the default dist +# normalize_rocm_dist.sh --apply # make the changes (writes a rollback script) +# normalize_rocm_dist.sh --check # invariant assertion; exit 1 if violated +# normalize_rocm_dist.sh --dist /path/to/rocm_devel [--apply|--check] +# +# --check is the CI-facing mode: cheap, read-only, and fails the build with a +# clear message instead of letting a crippled SDK produce a 2h39m hang. + +set -euo pipefail + +DIST="${ROCM_DIST:-/it-share/rccl-ci/rocm_devel}" +MODE="dry-run" + +while [[ $# -gt 0 ]]; do + case "$1" in + --apply) MODE="apply"; shift ;; + --check) MODE="check"; shift ;; + --dry-run) MODE="dry-run"; shift ;; + --dist) DIST="$2"; shift 2 ;; + -h|--help) sed -n '2,50p' "$0"; exit 0 ;; + *) echo "[ERROR] unknown argument: $1" >&2; exit 2 ;; + esac +done + +LIBDIR="${DIST%/}/lib" +[[ -d "${LIBDIR}" ]] || { echo "[ERROR] not a directory: ${LIBDIR}" >&2; exit 2; } + +# Resolve so the rollback script and log are unambiguous even if DIST is a symlink. +REAL_LIBDIR="$(readlink -f "${LIBDIR}")" + +echo "==========================================================================" +echo "ROCm dist SONAME normalisation" +echo " dist : ${DIST}" +echo " lib dir : ${REAL_LIBDIR}" +echo " mode : ${MODE}" +echo "==========================================================================" + +# --- discover groups of content-identical regular .so files ------------------- +# Only regular files: existing symlinks are already correct and must be left alone. +tmp_hashes="$(mktemp)" +trap 'rm -f "${tmp_hashes}"' EXIT + +find "${REAL_LIBDIR}" -maxdepth 1 -type f -name '*.so*' -exec md5sum {} + 2>/dev/null \ + | sed "s| ${REAL_LIBDIR}/| |" > "${tmp_hashes}" + +# Group by hash, emit "hash name1 name2 ..." for groups with >1 member. +groups="$(awk '{h=$1; sub(/^ +/,"",$2); g[h]=g[h]" "$2; n[h]++} + END {for (k in n) if (n[k]>1) print k g[k]}' "${tmp_hashes}" | sort)" + +if [[ -z "${groups}" ]]; then + echo "[OK] no duplicate-inode SONAME groups found — dist is already normalised." + exit 0 +fi + +# --- plan --------------------------------------------------------------------- +declare -a PLAN_LINK PLAN_TARGET +skipped=0 +bytes_freed=0 +groups_ok=0 + +while read -r _hash rest; do + # shellcheck disable=SC2206 + members=( ${rest} ) + + # Sort by name length ascending: libfoo.so, libfoo.so.1, libfoo.so.1.2.3 + mapfile -t sorted < <(printf '%s\n' "${members[@]}" | awk '{print length, $0}' | sort -n | cut -d' ' -f2-) + + # SAFETY GUARD: only touch a group whose names form a genuine versioned chain, + # i.e. each shorter name is a literal prefix of the next longer one. Two + # unrelated libraries that happen to be byte-identical (vendored duplicates, + # stubs) must NOT be collapsed into each other — that would silently rewrite + # the dependency graph. + chain_ok=1 + for (( i = 0; i < ${#sorted[@]} - 1; i++ )); do + if [[ "${sorted[i+1]}" != "${sorted[i]}"* ]]; then chain_ok=0; break; fi + done + + if [[ ${chain_ok} -eq 0 ]]; then + echo "[SKIP] not a versioned chain, leaving alone: ${sorted[*]}" + skipped=$(( skipped + 1 )) + continue + fi + + groups_ok=$(( groups_ok + 1 )) + canonical="${sorted[-1]}" + + # Build the conventional chain: each name points at the next longer name. + for (( i = 0; i < ${#sorted[@]} - 1; i++ )); do + PLAN_LINK+=( "${sorted[i]}" ) + PLAN_TARGET+=( "${sorted[i+1]}" ) + sz="$(stat -c %s "${REAL_LIBDIR}/${sorted[i]}" 2>/dev/null || echo 0)" + bytes_freed=$(( bytes_freed + sz )) + done + + printf ' %-42s <- %s\n' "${canonical}" "$(printf '%s ' "${sorted[@]::${#sorted[@]}-1}")" +done <<< "${groups}" + +echo +echo " chains to normalise : ${groups_ok}" +echo " files -> symlinks : ${#PLAN_LINK[@]}" +echo " groups skipped : ${skipped}" +printf " disk reclaimed : %.2f GB\n" "$(awk -v b="${bytes_freed}" 'BEGIN{print b/1024/1024/1024}')" +echo + +# --- check mode: assert the invariant, change nothing ------------------------- +if [[ "${MODE}" == "check" ]]; then + if [[ ${#PLAN_LINK[@]} -gt 0 ]]; then + echo "[FAIL] ${#PLAN_LINK[@]} duplicate-inode library file(s) in ${REAL_LIBDIR}." >&2 + echo " A dlopen() by unversioned name will load a SECOND, uninitialised" >&2 + echo " copy of these libraries. For ROCr this silently disables DMA-BUF" >&2 + echo " and hangs multi-node collectives." >&2 + echo " Fix: ${BASH_SOURCE[0]} --dist ${DIST} --apply" >&2 + exit 1 + fi + echo "[OK] invariant holds: one inode per SONAME." + exit 0 +fi + +if [[ "${MODE}" == "dry-run" ]]; then + echo "[DRY RUN] nothing changed. Re-run with --apply to make these changes." + exit 0 +fi + +# --- apply -------------------------------------------------------------------- +[[ -w "${REAL_LIBDIR}" ]] || { echo "[ERROR] ${REAL_LIBDIR} is not writable" >&2; exit 1; } + +stamp="$(date +%Y%m%d_%H%M%S)" +rollback="${REAL_LIBDIR}/.normalize_rollback_${stamp}.sh" +{ + echo "#!/usr/bin/env bash" + echo "# Undo normalize_rocm_dist.sh run of ${stamp}." + echo "# Replaces each symlink with an independent copy of its target, restoring" + echo "# the original (broken) multi-inode layout." + echo "set -euo pipefail" + echo "cd \"${REAL_LIBDIR}\"" +} > "${rollback}" + +converted=0 +for (( i = 0; i < ${#PLAN_LINK[@]}; i++ )); do + link="${PLAN_LINK[i]}" + target="${PLAN_TARGET[i]}" + + # Re-verify identical content at apply time. The plan was computed from a + # snapshot; refuse to act on anything that changed underneath us. + if ! cmp -s "${REAL_LIBDIR}/${link}" "${REAL_LIBDIR}/${target}"; then + echo "[SKIP] content diverged since planning: ${link}" >&2 + continue + fi + + echo "cp -a --remove-destination \"${target}\" \"${link}\"" >> "${rollback}" + + # Atomic replace: build the symlink under a temp name, then rename over the + # regular file. There is no instant where ${link} is absent, so a concurrent + # dlopen either gets the old file or the new symlink — never ENOENT. + ln -sfn "${target}" "${REAL_LIBDIR}/.${link}.tmp.$$" + mv -Tf "${REAL_LIBDIR}/.${link}.tmp.$$" "${REAL_LIBDIR}/${link}" + converted=$(( converted + 1 )) +done + +chmod +x "${rollback}" + +echo +echo "[OK] converted ${converted} file(s) to symlinks." +echo "[OK] rollback script: ${rollback}" + +# --- verify ------------------------------------------------------------------- +echo +echo "=== verification: one inode per SONAME chain ===" +fail=0 +while read -r _hash rest; do + # shellcheck disable=SC2206 + members=( ${rest} ) + inodes="$(for m in "${members[@]}"; do stat -Lc %i "${REAL_LIBDIR}/${m}" 2>/dev/null; done | sort -u | wc -l)" + if [[ "${inodes}" != "1" ]]; then + echo " [FAIL] ${members[*]} -> ${inodes} distinct inodes" + fail=1 + fi +done <<< "${groups}" + +[[ ${fail} -eq 0 ]] && echo " [OK] every normalised chain now resolves to a single inode." +exit "${fail}" diff --git a/ci/rccl_perf_gate/sbatch/lib/workspace.sh b/ci/rccl_perf_gate/sbatch/lib/workspace.sh new file mode 100755 index 000000000..480870160 --- /dev/null +++ b/ci/rccl_perf_gate/sbatch/lib/workspace.sh @@ -0,0 +1,550 @@ +#!/usr/bin/env bash +############################################################## +# Per-run workspace isolation for the RCCL A/B regression CI. +# +# WHY: the build and detect steps both write to fixed shared paths today — +# builds/{reference,candidate}/ librccl.so for each A/B side +# ab_artifacts/ ab_regression_report.json + rccl_runs.log +# cvs-sbatch/cluster.json regenerated in-tree on every run +# logs/latest global "most recent run" symlink +# With exactly one self-hosted runner those never overlap. The moment a second +# runner exists (see RCCL_CI_REMEDIATION_PLAN.md, issue #1) two PRs share them +# silently — worst case PR A's candidate librccl is measured against PR B's +# reference and the verdict is meaningless but looks legitimate. This module +# gives every run its own workspace so that cannot happen. +# +# OPT-IN: everything here is gated on RCCL_CI_WORKSPACE=1. Unset or 0 and every +# function is a no-op that returns success, so this file is safe to source +# unconditionally from the existing scripts and legacy shared-path behaviour is +# unchanged. +# +# LAYOUT (RCCL_CI_WORKSPACE=1): +# runs// +# meta.json provenance: run key, slurm ids, cvs sha, A/B revs, times +# config.json per-run detect config (lib paths + output_dir templated) +# cvs/ detached git worktree of cvs/ at the recorded sha +# cvs-sbatch/ real copy — run.sh regenerates cluster.json in-tree +# builds/ reference/ + candidate/ (see BUILD CACHE below) +# artifacts/ detector output_dir +# logs/ RUN_LOG_DIR +# +# RUN KEY: must be stable across the build job and the detect job, because +# detect loads the librccl that build produced. The Slurm job id differs between +# them, so the key is GITHUB_RUN_ID (+ attempt), falling back to SLURM_JOB_ID for +# manual sbatch and a timestamp for bare local runs. +# +# BUILD CACHE: build_rccl.sh already has a rev-aware skip (lib/.built_rev), so +# naively giving every run a private builds/ would turn every run into a cold +# ~30min LTO build and make the pipeline SLOWER — the opposite of the goal. +# Instead librccl is cached by content under builds/by-rev/-/ and +# the per-run builds//lib is populated from it by hardlink. A given git rev +# built with a given recipe is immutable, so sharing across runs is safe, and the +# merge-base reference is usually identical across PRs targeting develop — so +# most runs hit cache on BOTH sides. Publishing is atomic (build into a private +# tmp dir, then rename), so two runs racing on the same rev cannot corrupt it; +# the loser just adopts the winner's copy. +# Set RCCL_CI_BUILD_CACHE=0 for a fully private cold build per run instead. +############################################################## + +# Deliberately no `set -euo pipefail` here — this file is sourced by callers that +# already set it, and we don't want to impose it on any that don't. + +# Assign only when unset. Callers (run_rccl_build.sh, run_rccl_ab.sh) declare +# this readonly BEFORE sourcing us, and reassigning a readonly variable is a +# fatal error under 'set -e' -- which would break them even with the workspace +# flag OFF, since sourcing happens before any ws_enabled check. +if [[ -z "${RCCL_CI_ROOT:-}" ]]; then + RCCL_CI_ROOT="/it-share/rccl-ci" +fi + +# How many completed run workspaces the janitor keeps, and the age floor below +# which a workspace is never reaped regardless of count (so a run that is still +# in flight, or one someone is actively debugging, survives). +RCCL_CI_WS_KEEP_RUNS="${RCCL_CI_WS_KEEP_RUNS:-20}" +RCCL_CI_WS_KEEP_DAYS="${RCCL_CI_WS_KEEP_DAYS:-14}" + +ws_log() { echo "[workspace $(date +%H:%M:%S)] $*"; } +ws_warn() { echo "[workspace WARN] $*" >&2; } + +# --------------------------------------------------------------------------- +# ws_enabled — the single gate. Every other function short-circuits on this. +# --------------------------------------------------------------------------- +# Default ON as of the workspace rollout. It was introduced opt-in so it could be +# validated against the live pipeline without risking prod; that validation +# passed (build + 4-node detect, 224 sweeps, prod config verifiably untouched), +# and leaving it opt-in would mean the shared-path clobbering returns the moment +# a second runner is added. Set RCCL_CI_WORKSPACE=0 to fall back to the legacy +# shared trees for a one-off debug run. +ws_enabled() { + [[ "${RCCL_CI_WORKSPACE:-1}" == "1" ]] +} + +ws_cache_enabled() { + [[ "${RCCL_CI_BUILD_CACHE:-1}" == "1" ]] +} + +# --------------------------------------------------------------------------- +# ws_run_key — stable identity shared by the build job and the detect job. +# +# GITHUB_RUN_ID is the only id both Slurm jobs of one PR run see, so it is the +# primary key. RUN_ATTEMPT is included because a re-run of a failed workflow +# should get a clean workspace rather than inherit half-written state. +# --------------------------------------------------------------------------- +ws_run_key() { + if [[ -n "${RCCL_CI_RUN_KEY:-}" ]]; then + echo "${RCCL_CI_RUN_KEY}" + elif [[ -n "${GITHUB_RUN_ID:-}" ]]; then + echo "gh-${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT:-1}" + elif [[ -n "${SLURM_JOB_ID:-}" ]]; then + echo "slurm-${SLURM_JOB_ID}" + else + echo "local-$(date +%Y%m%d_%H%M%S)-$$" + fi +} + +ws_root() { + echo "${RCCL_CI_ROOT}/runs/$(ws_run_key)" +} + +# --------------------------------------------------------------------------- +# ws_recipe_hash — invalidate the build cache when the toolchain changes. +# +# build_rccl.sh stamps only the git rev in .built_rev, so a lib built with an +# older ROCm dist or a different gfx target would look like a cache hit. Fold the +# things that actually change codegen into the cache key. ROCM_DIST is resolved +# through its symlink on purpose: rocm_devel -> 7.14.0a/... so a dist bump +# changes the hash even though the symlink path is constant. +# --------------------------------------------------------------------------- +ws_recipe_hash() { + local rocm_dist gpu_targets resolved + rocm_dist="${ROCM_DIST:-${RCCL_CI_ROOT}/rocm_devel}" + gpu_targets="${GPU_TARGETS:-gfx950}" + resolved="$(readlink -f "${rocm_dist}" 2>/dev/null || echo "${rocm_dist}")" + printf '%s|%s' "${resolved}" "${gpu_targets}" | sha256sum | cut -c1-12 +} + +ws_cache_dir_for() { + # $1 = git rev + local rev="$1" + echo "${RCCL_CI_ROOT}/builds/by-rev/${rev}-$(ws_recipe_hash)" +} + +# --------------------------------------------------------------------------- +# ws_init — create the workspace. Idempotent: the build job calls it first, the +# detect job calls it again later and must find the same tree intact. +# --------------------------------------------------------------------------- +ws_init() { + ws_enabled || return 0 + + local ws cvs_src cvs_sha + ws="$(ws_root)" + cvs_src="${RCCL_CI_ROOT}/cvs" + + mkdir -p "${ws}"/{builds,artifacts,logs} || { + ws_warn "could not create workspace at ${ws}" + return 1 + } + + # --- cvs: detached git worktree ------------------------------------------ + # --detach is required: the branch (aimvt-196-rccl-regression-robustness) is + # already checked out in the main worktree and git refuses a second checkout + # of the same branch. Detaching at the resolved sha also gives us exact + # provenance — meta.json records which cvs revision produced the verdict. + if [[ ! -d "${ws}/cvs" ]]; then + local cvs_dirty="" + cvs_sha="$(git -C "${cvs_src}" rev-parse HEAD 2>/dev/null)" + [[ -n "${cvs_sha}" ]] && cvs_dirty="$(git -C "${cvs_src}" status --porcelain 2>/dev/null | head -1)" + + if [[ -z "${cvs_sha}" ]]; then + ws_warn "cvs is not a git checkout; falling back to a plain copy" + cp -a "${cvs_src}" "${ws}/cvs" || return 1 + cvs_sha="unknown" + elif [[ -n "${cvs_dirty}" ]]; then + # A worktree checks out HEAD, so uncommitted changes in cvs/ would be + # SILENTLY DROPPED — the run would execute different code than the tree + # the operator is looking at, and meta.json would record a sha that does + # not describe what ran. Copy instead: correctness beats saving 280M. + ws_warn "cvs has uncommitted changes — using a full copy, not a worktree." + ws_warn " Commit them to get the cheap worktree and exact provenance back." + cp -a "${cvs_src}" "${ws}/cvs" || return 1 + cvs_sha="${cvs_sha}-dirty" + else + # Concurrent worktree adds contend on the repo lock; retry briefly. + local attempt + for attempt in 1 2 3; do + if git -C "${cvs_src}" worktree add --detach "${ws}/cvs" "${cvs_sha}" >/dev/null 2>&1; then + break + fi + [[ ${attempt} -eq 3 ]] && { ws_warn "git worktree add failed after 3 attempts"; return 1; } + sleep $(( attempt * 2 )) + done + fi + else + cvs_sha="$(git -C "${ws}/cvs" rev-parse HEAD 2>/dev/null || echo unknown)" + fi + + # --- cvs-sbatch: real copy ------------------------------------------------ + # Must be a real copy, not a link: run.sh's generate_cluster_config rewrites + # cluster.json in-tree, which is precisely the file two concurrent runs would + # corrupt for each other. + if [[ ! -d "${ws}/cvs-sbatch" ]]; then + cp -a "${RCCL_CI_ROOT}/cvs-sbatch" "${ws}/cvs-sbatch" || return 1 + fi + + ws_write_meta "${cvs_sha}" + + ws_log "workspace ready: ${ws} (cvs ${cvs_sha:0:10})" + return 0 +} + +# --------------------------------------------------------------------------- +# ws_write_meta — provenance. Merged, not overwritten, so the detect job adds +# its Slurm id without erasing the build job's. +# --------------------------------------------------------------------------- +ws_write_meta() { + local cvs_sha="${1:-unknown}" + local ws meta + ws="$(ws_root)" + meta="${ws}/meta.json" + + RCCL_WS_META="${meta}" \ + RCCL_WS_KEY="$(ws_run_key)" \ + RCCL_WS_CVS_SHA="${cvs_sha}" \ + RCCL_WS_RECIPE="$(ws_recipe_hash)" \ + python3 - <<'PY' 2>/dev/null || true +import json, os, datetime + +path = os.environ["RCCL_WS_META"] +try: + with open(path) as fh: + doc = json.load(fh) +except Exception: + doc = {} + +doc.setdefault("run_key", os.environ["RCCL_WS_KEY"]) +doc.setdefault("created_utc", datetime.datetime.utcnow().isoformat() + "Z") +doc["updated_utc"] = datetime.datetime.utcnow().isoformat() + "Z" +doc["cvs_sha"] = os.environ["RCCL_WS_CVS_SHA"] +doc["recipe_hash"] = os.environ["RCCL_WS_RECIPE"] + +for key, env in ( + ("github_run_id", "GITHUB_RUN_ID"), + ("github_run_attempt", "GITHUB_RUN_ATTEMPT"), + ("github_sha", "GITHUB_SHA"), + ("github_pr", "GITHUB_PR_NUMBER"), +): + if os.environ.get(env): + doc[key] = os.environ[env] + +# Slurm job ids accumulate: one for the build job, one for detect. +job = os.environ.get("SLURM_JOB_ID") +if job: + name = os.environ.get("SLURM_JOB_NAME", "job") + jobs = doc.setdefault("slurm_jobs", {}) + jobs[name] = job + +with open(path, "w") as fh: + json.dump(doc, fh, indent=2, sort_keys=True) + fh.write("\n") +PY +} + +# --------------------------------------------------------------------------- +# ws_record — stash an arbitrary key/value in meta.json (revs, verdicts, ...). +# --------------------------------------------------------------------------- +ws_record() { + ws_enabled || return 0 + local key="$1" value="$2" + RCCL_WS_META="$(ws_root)/meta.json" RCCL_WS_K="${key}" RCCL_WS_V="${value}" \ + python3 - <<'PY' 2>/dev/null || true +import json, os +path = os.environ["RCCL_WS_META"] +try: + with open(path) as fh: + doc = json.load(fh) +except Exception: + doc = {} +doc[os.environ["RCCL_WS_K"]] = os.environ["RCCL_WS_V"] +with open(path, "w") as fh: + json.dump(doc, fh, indent=2, sort_keys=True) + fh.write("\n") +PY +} + +# --------------------------------------------------------------------------- +# ws_cache_fetch