diff --git a/mlpstorage_py/submission_checker/checks/checkpointing_checks.py b/mlpstorage_py/submission_checker/checks/checkpointing_checks.py index a855c48b..04959c20 100644 --- a/mlpstorage_py/submission_checker/checks/checkpointing_checks.py +++ b/mlpstorage_py/submission_checker/checks/checkpointing_checks.py @@ -1006,9 +1006,14 @@ def simultaneous_rw_support(self): ``SCHEMA_ERROR_RULE_MAP`` tagged with 4.7.4 — those catch declaration-side defects that surface before any benchmark runs). - The rule body preserves the ``@rule`` binding for coverage - discovery and emits an INFO line so tooling that greps by rule - ID surfaces the rule as "visited and satisfied". + Warnings-only enforcement (never fails — submission-window + doctrine): a completed checkpointing run has already demonstrated + simultaneous R/W on the shared namespace via the CAP-02 probe, so a + system description that declares either capability as ``false`` + CONTRADICTS the run it accompanies. The validator surfaces that + inconsistency to reviewers via ``warn_violation`` and still returns + valid. Missing capability fields are owned by SystemYamlSchemaCheck + (D-A3) and are silent-skipped here to avoid double-emit. """ valid = True if self.mode != "checkpointing": @@ -1016,7 +1021,28 @@ def simultaneous_rw_support(self): sim_write = self._get_capability("simultaneous_write") sim_read = self._get_capability("simultaneous_read") if sim_write is None or sim_read is None: + # SystemYamlSchemaCheck owns the missing-field violation (D-A3). return valid + # An explicitly-declared ``false`` contradicts the completed run. + denied = [ + name + for name, value in ( + ("simultaneous_write", sim_write), + ("simultaneous_read", sim_read), + ) + if value is False + ] + if denied: + self.warn_violation( + "4.7.4", "checkpointSimultaneousRwSupport", self.path, + "system YAML declares %s = false, but a completed " + "checkpointing run requires simultaneous read/write on a " + "shared namespace (Rules.md 4.7.4); the system description " + "is likely incorrect.", + " and ".join(denied), + ) + return valid + # Both capabilities declared-supported — satisfied by construction. self.log.info( "[4.7.4 checkpointSimultaneousRwSupport] %s: " "satisfied by construction — CAP-02 shared-FS probe " diff --git a/mlpstorage_py/submission_checker/checks/training_checks.py b/mlpstorage_py/submission_checker/checks/training_checks.py index 92cd7062..71e95642 100644 --- a/mlpstorage_py/submission_checker/checks/training_checks.py +++ b/mlpstorage_py/submission_checker/checks/training_checks.py @@ -692,34 +692,81 @@ def identical_accelerators_per_node(self): return valid + # Above this per-host max/min capability ratio, the physical nodes are + # "widely enough different" to warrant reviewer attention per Rules.md + # 3.3.7. Advisory only — a heterogeneous cluster is not itself illegal. + _NODE_CAPABILITY_DIVERGENCE_RATIO = 1.5 + @rule("3.3.7", "trainingNodeCapabilityConsistency") def node_capability_consistency_check(self): - """Rules.md 3.3.7 — node capability consistency (advisory). - - Satisfied by construction at runtime: the cluster collector - captures per-host system information twice per run — once at - run start (``Benchmark._collect_cluster_start`` in - ``benchmarks/base.py:646``) and once at run end - (``_collect_cluster_end`` at ``:674``) — and stores both - snapshots in ``metadata['cluster_snapshots']`` via - ``rules.models.ClusterSnapshots.as_dict()``. Any component drift - during the run (CPU count, memory, network ports, kernel, etc.) - is therefore captured in the artifact tree; the cluster collector - also emits a warning on significant drift so operators can spot - stability issues before submission. - - The rule body preserves the ``@rule`` binding for coverage - discovery and emits an INFO line so tooling that greps by rule - ID surfaces the rule as "visited and satisfied". + """Rules.md 3.3.7 — node capability consistency (warnings-only). + + Rules.md 3.3.7 asks the validator to **warn (not fail)** when the + physical nodes in a distributed submission differ widely in + capability. Each run's ``metadata['cluster_information']`` (the + ``ClusterInformation.as_dict`` snapshot) carries per-host memory and + CPU-core vectors plus any collector-recorded + ``host_consistency_issues``. This check assesses the first run that + reports a usable multi-host cluster and emits a + ``warn_violation`` when the per-host max/min ratio for memory or CPU + cores exceeds ``_NODE_CAPABILITY_DIVERGENCE_RATIO``, or when the + collector already flagged inconsistencies. It always returns valid — + node heterogeneity is advisory, never a hard failure (submission- + window doctrine). + + No cluster_information, or a single-host cluster, is silently + passed — there is nothing to compare. """ - self.log.info( - "[3.3.7 trainingNodeCapabilityConsistency] %s: " - "satisfied by construction — cluster collector captures " - "start/end cluster snapshots (Benchmark._collect_cluster_start / " - "_collect_cluster_end); component drift is surfaced at runtime", - self.path, - ) - return True + valid = True + if self.mode != "training": + return valid + + for _summary, metadata, ts in self.submissions_logs.run_files: + if metadata is None: + continue + cluster = metadata.get("cluster_information") + if not isinstance(cluster, dict): + continue + hosts = cluster.get("hosts") + if not isinstance(hosts, list) or len(hosts) < 2: + continue + + # Per-host capability vectors. Missing/partial fields are simply + # excluded from the comparison for that metric. + mem_totals = [ + (h.get("memory") or {}).get("total") + for h in hosts if isinstance(h, dict) + ] + core_counts = [ + (h.get("cpu") or {}).get("num_cores") + for h in hosts if isinstance(h, dict) + ] + for label, values in (("memory", mem_totals), ("CPU cores", core_counts)): + nums = [v for v in values if isinstance(v, (int, float)) and v > 0] + if len(nums) < 2: + continue + lo, hi = min(nums), max(nums) + if hi / lo > self._NODE_CAPABILITY_DIVERGENCE_RATIO: + self.warn_violation( + "3.3.7", "trainingNodeCapabilityConsistency", self.path, + "run %s: hosts differ widely in %s — min=%s, max=%s " + "(%.2fx); confirm the physical nodes are intended to " + "be heterogeneous (Rules.md 3.3.7).", + ts, label, lo, hi, hi / lo, + ) + + issues = cluster.get("host_consistency_issues") + if issues: + self.warn_violation( + "3.3.7", "trainingNodeCapabilityConsistency", self.path, + "run %s: cluster collector flagged host inconsistencies: %s", + ts, "; ".join(str(i) for i in issues), + ) + + # Node capability is a property of the physical cluster, identical + # across the measured runs — one representative run suffices. + return valid + return valid @rule("3.6.1", "trainingClosedSubmissionChecksum") def closed_submission_checksum(self): diff --git a/mlpstorage_py/tests/conftest.py b/mlpstorage_py/tests/conftest.py index 60c42e07..327862ca 100644 --- a/mlpstorage_py/tests/conftest.py +++ b/mlpstorage_py/tests/conftest.py @@ -459,6 +459,7 @@ def build_submission(tmp_path, **overrides) -> Path: run_logfile_df_block = overrides.pop("run_logfile_df_block", None) chkpt_logfile_df_block = overrides.pop("chkpt_logfile_df_block", None) run_metadata_hosts = overrides.pop("run_metadata_hosts", None) + run_metadata_cluster_information = overrides.pop("run_metadata_cluster_information", None) chkpt_summary_timestamps = overrides.pop("chkpt_summary_timestamps", False) chkpt_split_mode = overrides.pop("chkpt_split_mode", False) chkpt_cache_flush_gap_seconds = overrides.pop("chkpt_cache_flush_gap_seconds", 25) @@ -706,6 +707,11 @@ def build_submission(tmp_path, **overrides) -> Path: run_meta["args"]["data_dir"] = run_data_dir if run_results_dir is not None: run_meta["args"]["results_dir"] = run_results_dir + # 3.3.7: inject a cluster_information block (ClusterInformation + # .as_dict shape) so node-capability-consistency tests can + # exercise divergent / uniform per-host capability vectors. + if run_metadata_cluster_information is not None: + run_meta["cluster_information"] = run_metadata_cluster_information (ts_dir / "metadata.json").write_text( json.dumps(run_meta), encoding="utf-8" ) diff --git a/mlpstorage_py/tests/test_checkpointing_check_phase2.py b/mlpstorage_py/tests/test_checkpointing_check_phase2.py index 4e7bd26e..ad8fea44 100644 --- a/mlpstorage_py/tests/test_checkpointing_check_phase2.py +++ b/mlpstorage_py/tests/test_checkpointing_check_phase2.py @@ -595,18 +595,27 @@ class TestChkpt05_SimultaneousRwSupport: """ def test_both_true_no_emission(self, tmp_path, mock_logger): - """Default capabilities (sim_write=True, sim_read=True) → True, no errors, info emitted.""" + """Default capabilities (sim_write=True, sim_read=True) → True, no errors/warnings, info emitted.""" from mlpstorage_py.tests.conftest import build_submission root = build_submission(tmp_path) check = _run_checkpointing_check(root, mock_logger) result = check.simultaneous_rw_support() assert result is True assert mock_logger.errors == [] - # Default: emits info with the "satisfied by construction" marker + # Both capabilities declared-supported → satisfied by construction: + # info emitted, and NO false-positive warning. + assert mock_logger.warnings == [] assert len(mock_logger.infos) >= 1 - def test_sim_write_false_emits_info(self, tmp_path, mock_logger): - """sim_write=False → returns True, emits info with [4.7.4 ...] prefix.""" + def test_sim_write_false_emits_warning(self, tmp_path, mock_logger): + """sim_write=False → warnings-only: returns True, emits a [4.7.4 ...] WARNING. + + A completed checkpointing run has already demonstrated simultaneous + R/W via the CAP-02 runtime probe, so a system description that + declares simultaneous_write=false contradicts the run it accompanies + (Rules.md 4.7.4). Surface that inconsistency to reviewers as a + warning — never a hard failure. + """ from mlpstorage_py.tests.conftest import build_submission root = build_submission( tmp_path, @@ -618,14 +627,17 @@ def test_sim_write_false_emits_info(self, tmp_path, mock_logger): assert mock_logger.errors == [] assert any( m.startswith("[4.7.4 checkpointSimultaneousRwSupport]") - for m in mock_logger.infos - ), f"Expected info starting with [4.7.4 ...]; got {mock_logger.infos}" + and "simultaneous_write" in m + for m in mock_logger.warnings + ), f"Expected a warning starting with [4.7.4 ...]; got {mock_logger.warnings}" def test_missing_field_silently_passes(self, tmp_path, mock_logger): - """Missing simultaneous_write field → silent-pass per D-A3. + """Missing simultaneous_write field → silent-pass, NO warning, per D-A3. - SystemYamlSchemaCheck handles the missing-field violation. - CHKPT-05 silent-skips when _get_capability returns None. + SystemYamlSchemaCheck owns the missing-field violation; 4.7.4 + silent-skips when _get_capability returns None to avoid double-emit. + The warnings-only inconsistency check fires only for an explicitly + declared ``false``, not for an absent field. """ from mlpstorage_py.tests.conftest import build_submission root = build_submission( @@ -637,6 +649,7 @@ def test_missing_field_silently_passes(self, tmp_path, mock_logger): result = check.simultaneous_rw_support() assert result is True assert mock_logger.errors == [] + assert mock_logger.warnings == [] # --------------------------------------------------------------------------- @@ -880,13 +893,13 @@ class TestChkpt05SatisfiedByConstruction: check trips these assertions. """ - def test_sim_write_false_emits_info_with_satisfied_marker(self, tmp_path, mock_logger): - """sim_write=False → log.info with 'satisfied by construction' marker; no errors. + def test_sim_write_false_emits_warning_not_error(self, tmp_path, mock_logger): + """sim_write=False → a [4.7.4 ...] WARNING (not an error, not an info-satisfied line). - The rule doesn't inspect the capability values to decide pass/fail — - the runtime gate (CAP-02) has already enforced the invariant before - the validator runs. The capability values are still logged for - auditability. + The rule never fails the submission (warnings-only): the runtime + gate (CAP-02) already enforced the invariant. But a declared + simultaneous_write=false contradicts that, so the validator surfaces + it to reviewers as a warning rather than mislabelling it "satisfied". """ from mlpstorage_py.tests.conftest import build_submission root = build_submission( @@ -897,23 +910,23 @@ def test_sim_write_false_emits_info_with_satisfied_marker(self, tmp_path, mock_l result = check.simultaneous_rw_support() assert result is True assert mock_logger.errors == [], \ - f"CHKPT-05 must not emit errors (satisfied by CAP-02 at runtime); got {mock_logger.errors}" - matching_infos = [ - m for m in mock_logger.infos + f"4.7.4 must not emit errors (warnings-only); got {mock_logger.errors}" + matching_warnings = [ + m for m in mock_logger.warnings if "[4.7.4 checkpointSimultaneousRwSupport]" in m - and "satisfied by construction" in m ] - assert len(matching_infos) >= 1, ( - f"Expected at least one info message with [4.7.4 ...] and " - f"'satisfied by construction' marker; got infos: {mock_logger.infos}" + assert len(matching_warnings) >= 1, ( + f"Expected at least one warning with [4.7.4 ...]; " + f"got warnings: {mock_logger.warnings}" ) def test_returns_true_regardless_of_capability_values(self, tmp_path, mock_logger): - """CHKPT-05 returns True regardless of declared capability values. + """4.7.4 returns True regardless of declared capability values (warnings-only). The runtime gate (CAP-02 shared-FS probe) is the authoritative - enforcement point; declared simultaneous_write / simultaneous_read - values are audit-only from the validator's perspective. + enforcement point, so the validator never fails. When both + simultaneous_write and simultaneous_read are declared false, it emits + a warning naming both but still returns True. """ from mlpstorage_py.tests.conftest import build_submission root = build_submission( @@ -923,4 +936,10 @@ def test_returns_true_regardless_of_capability_values(self, tmp_path, mock_logge check = _run_checkpointing_check(root, mock_logger) result = check.simultaneous_rw_support() assert result is True, \ - "CHKPT-05 must return True (satisfied by CAP-02) even when both sim_* are False" + "4.7.4 must return True (warnings-only) even when both sim_* are False" + assert mock_logger.errors == [] + assert any( + "[4.7.4 checkpointSimultaneousRwSupport]" in m + and "simultaneous_write" in m and "simultaneous_read" in m + for m in mock_logger.warnings + ), f"Expected a warning naming both fields; got {mock_logger.warnings}" diff --git a/mlpstorage_py/tests/test_training_check_retrofit.py b/mlpstorage_py/tests/test_training_check_retrofit.py index 72dd3999..f39e9842 100644 --- a/mlpstorage_py/tests/test_training_check_retrofit.py +++ b/mlpstorage_py/tests/test_training_check_retrofit.py @@ -193,35 +193,120 @@ def test_3_3_5_distributed_data_accessibility_logs_info_returns_true(tmp_path, m # --------------------------------------------------------------------------- -# Satisfied-by-construction: 3.3.7 node_capability_consistency_check +# 3.3.7 node_capability_consistency_check — warnings-only node divergence # --------------------------------------------------------------------------- -def test_3_3_7_node_capability_consistency_logs_info_returns_true(tmp_path, mock_logger): - """3.3.7 is satisfied by construction at runtime via cluster start/end - snapshots (Benchmark._collect_cluster_start / _collect_cluster_end). - Any component drift during the run is captured in - metadata['cluster_snapshots'] and surfaced to the operator. +_GB = 1024 ** 3 - The rule body preserves the @rule binding and emits a single INFO - line carrying the "satisfied by construction" marker. - """ + +def _cluster_info(mem_totals, core_counts, consistency_issues=None): + """Build a ClusterInformation.as_dict-shaped block with per-host vectors.""" + hosts = [ + { + "hostname": f"host{i}", + "memory": {"total": mem}, + "cpu": {"num_cores": cores}, + } + for i, (mem, cores) in enumerate(zip(mem_totals, core_counts)) + ] + ci = { + "num_hosts": len(hosts), + "min_memory_bytes": min(mem_totals), + "max_memory_bytes": max(mem_totals), + "hosts": hosts, + } + if consistency_issues: + ci["host_consistency_issues"] = consistency_issues + return ci + + +def _3_3_7_warnings(mock_logger): + return [ + m for m in mock_logger.warnings + if m.startswith("[3.3.7 trainingNodeCapabilityConsistency]") + ] + + +def test_3_3_7_no_cluster_information_silent_pass(tmp_path, mock_logger): + """No cluster_information to assess → returns True, no warning/error.""" from mlpstorage_py.tests.conftest import build_submission root = build_submission(tmp_path) check = _run_training_check(root, mock_logger) result = check.node_capability_consistency_check() - assert result is True, f"expected True; got {result!r}" - matching = [ - m for m in mock_logger.infos - if m.startswith("[3.3.7 trainingNodeCapabilityConsistency]") - ] - assert matching, ( - f"expected info starting with [3.3.7 trainingNodeCapabilityConsistency]; " - f"got infos={mock_logger.infos}" + assert result is True + assert mock_logger.errors == [] + assert _3_3_7_warnings(mock_logger) == [] + + +def test_3_3_7_uniform_hosts_no_warning(tmp_path, mock_logger): + """Hosts with matching memory + cores → no divergence warning.""" + from mlpstorage_py.tests.conftest import build_submission + root = build_submission( + tmp_path, + run_metadata_cluster_information=_cluster_info( + [256 * _GB, 256 * _GB], [64, 64] + ), ) - assert any("satisfied by construction" in m for m in matching), ( - f"expected 'satisfied by construction' in 3.3.7 info line; got {matching}" + check = _run_training_check(root, mock_logger) + result = check.node_capability_consistency_check() + assert result is True + assert mock_logger.errors == [] + assert _3_3_7_warnings(mock_logger) == [] + + +def test_3_3_7_warns_on_wide_memory_divergence(tmp_path, mock_logger): + """Hosts differing 2x in memory → warns (never fails).""" + from mlpstorage_py.tests.conftest import build_submission + root = build_submission( + tmp_path, + run_metadata_cluster_information=_cluster_info( + [256 * _GB, 512 * _GB], [64, 64] + ), + ) + check = _run_training_check(root, mock_logger) + result = check.node_capability_consistency_check() + assert result is True + assert mock_logger.errors == [] + warns = _3_3_7_warnings(mock_logger) + assert any("memory" in m for m in warns), ( + f"expected a memory-divergence warning; got {mock_logger.warnings}" + ) + + +def test_3_3_7_warns_on_wide_cpu_divergence(tmp_path, mock_logger): + """Hosts differing 2x in CPU cores → warns (never fails).""" + from mlpstorage_py.tests.conftest import build_submission + root = build_submission( + tmp_path, + run_metadata_cluster_information=_cluster_info( + [256 * _GB, 256 * _GB], [32, 64] + ), + ) + check = _run_training_check(root, mock_logger) + result = check.node_capability_consistency_check() + assert result is True + assert mock_logger.errors == [] + warns = _3_3_7_warnings(mock_logger) + assert any("CPU cores" in m for m in warns), ( + f"expected a CPU-core-divergence warning; got {mock_logger.warnings}" + ) + + +def test_3_3_7_surfaces_host_consistency_issues(tmp_path, mock_logger): + """Collector-recorded host_consistency_issues are surfaced as a warning.""" + from mlpstorage_py.tests.conftest import build_submission + root = build_submission( + tmp_path, + run_metadata_cluster_information=_cluster_info( + [256 * _GB, 256 * _GB], [64, 64], + consistency_issues=["kernel version mismatch across hosts"], + ), + ) + check = _run_training_check(root, mock_logger) + result = check.node_capability_consistency_check() + assert result is True + assert mock_logger.errors == [] + warns = _3_3_7_warnings(mock_logger) + assert any("kernel version mismatch" in m for m in warns), ( + f"expected host-consistency-issue warning; got {mock_logger.warnings}" ) - assert not any( - m.startswith("[3.3.7 ") - for m in mock_logger.errors - ), f"3.3.7 emitted error: {mock_logger.errors}"