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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 24 additions & 8 deletions src/inference_endpoint/config/ruleset_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

from typing import TYPE_CHECKING

from .rulesets.mlcommons.rules import ALL_ROUNDS as mlcommons_rounds
from .rulesets.mlcommons.rules import CURRENT as mlcommons_current
from .rulesets.mlcommons.rules import EDGE_CURRENT as mlcommons_edge_current

Expand All @@ -44,7 +45,16 @@ def register_ruleset(name: str, ruleset: BenchmarkSuiteRuleset) -> None:
Args:
name: Unique name/version identifier (e.g., "mlperf-inference-v5.1")
ruleset: The ruleset instance to register

Raises:
ValueError: If a ruleset is already registered under ``name`` (prevents
an accidental overwrite via this function). The MLCommons
auto-registration path uses ``setdefault`` and does not hit this
guard; round-version uniqueness is enforced by
``test_round_versions_are_unique``.
"""
if name in _RULESET_REGISTRY:

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

[Claude] low (api-contract): This guard makes the module non-idempotent — _auto_register_mlcommons() runs at import time, so a second call or an importlib.reload of this module (previously a silent overwrite) now raises ValueError mid-import. No current caller reloads, so nothing breaks today, but reload-safety is gone. If that matters, use dict.setdefault/an in-check for the auto-register path while keeping the raise for external callers.

raise ValueError(f"Ruleset {name!r} is already registered")
_RULESET_REGISTRY[name] = ruleset


Expand Down Expand Up @@ -79,16 +89,22 @@ def list_rulesets() -> list[str]:

# Auto-register MLCommons rulesets
def _auto_register_mlcommons():
"""Auto-register MLCommons rulesets."""
# Register with version-specific name
register_ruleset(f"mlperf-inference-{mlcommons_current.version}", mlcommons_current)
# Also register as "mlcommons-current" for convenience
register_ruleset("mlcommons-current", mlcommons_current)
# Edge-Agentic (BFCL v4) ruleset
register_ruleset(
"""Auto-register MLCommons rulesets (idempotent).

Uses ``setdefault`` so re-running (e.g. a module re-import) is a no-op
rather than tripping ``register_ruleset``'s duplicate guard. Uniqueness of
round versions is enforced separately by ``test_round_versions_are_unique``.
"""
# Register every known round under its version-specific name
for ruleset in mlcommons_rounds:
_RULESET_REGISTRY.setdefault(f"mlperf-inference-{ruleset.version}", ruleset)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

[Claude] medium / design: setdefault bypasses the duplicate guard added to register_ruleset in this same PR. If two entries in ALL_ROUNDS ever share a .version, the second is silently dropped (first-wins) with no error — the ValueError guard at line 53 is unreachable from this primary registration path. Production correctness then rests solely on the test_round_versions_are_unique unit test being run. Consider registering rounds through register_ruleset (so duplicates fail loudly at import) and handling re-import idempotency with a sentinel, rather than trading a runtime invariant for a test-only one.

# Also register the current round as "mlcommons-current" for convenience
_RULESET_REGISTRY.setdefault("mlcommons-current", mlcommons_current)
# Edge-agentic ruleset: by version and as the edge "current".
_RULESET_REGISTRY.setdefault(
f"mlperf-{mlcommons_edge_current.version}", mlcommons_edge_current
) # -> "mlperf-edge-v0.1"
register_ruleset("mlperf-edge-current", mlcommons_edge_current)
_RULESET_REGISTRY.setdefault("mlperf-edge-current", mlcommons_edge_current)


# Auto-register on import
Expand Down
26 changes: 25 additions & 1 deletion src/inference_endpoint/config/rulesets/mlcommons/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
https://github.com/mlcommons/inference_policies/blob/master/inference_rules.adoc
"""

import copy
import random
from dataclasses import dataclass, field
from enum import Enum
Expand Down Expand Up @@ -272,7 +273,30 @@ def apply_user_config(
)


CURRENT = _v5_1
# v6.1 per-model latency targets are unchanged from v5.1 (they match the
# current mlperf.conf); only the round RNG seeds rotate. Seeds are the
# schedule_rng_seed / sample_index_rng_seed values from loadgen/mlperf.conf,
# pinned to a specific upstream commit for traceability:
# https://github.com/mlcommons/inference/blob/10f823448fd38bb739e52690efe8191c3a55412b/loadgen/mlperf.conf#L42-L43
# qsl_rng_seed is intentionally not modeled: it only selects the load order of
# the first-N working set, which the sample-index shuffle already covers.
_v6_1 = RoundRuleset(
version="v6.1",
scheduler_rng_seed=16159082839903944936,
sample_index_rng_seed=2747215439041700203,
benchmark_rulesets={
# Keep the model-singleton keys; deep-copy only the per-model rules so
# v6.1's mutable leaves (e.g. reported_metrics) can't leak into v5.1.
model: copy.deepcopy(rules)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

[Claude] low / design: Deep-copy correctness rides on enum-deepcopy identity. copy.deepcopy(rules) recurses into the inner dict's keys; lookups like benchmark_rulesets[model][opt_prio] keep working only because OptimizationPriority is an Enum and copy.deepcopy(member) is member. If it ever became a plain class without __deepcopy__, every apply_user_config lookup with the module-level singleton would KeyError. Prefer {k: copy.deepcopy(v) for k, v in rules.items()} to preserve keys explicitly. (The deep copy is also purely defensive — nothing currently mutates reported_metrics — so a key-preserving shallow copy would be equally safe and clearer about intent.)

for model, rules in _v5_1.benchmark_rulesets.items()
},
)


# All known rounds, registered by version in the ruleset registry.
ALL_ROUNDS = [_v5_1, _v6_1]

CURRENT = _v6_1


# --- Edge-Agentic (BFCL v4) ruleset ---
Expand Down
62 changes: 62 additions & 0 deletions src/inference_endpoint/config/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

from __future__ import annotations

import logging
from collections import Counter
from enum import Enum
from pathlib import Path
Expand Down Expand Up @@ -50,6 +51,8 @@
from .ruleset_base import BenchmarkSuiteRuleset
from .utils import parse_dataset_string, resolve_env_vars

logger = logging.getLogger(__name__)


class SystemDefaults(BaseModel):
DEFAULT_TIMEOUT: ClassVar[float] = 300.0
Expand Down Expand Up @@ -1029,8 +1032,67 @@ def _resolve_and_validate(self) -> Self:
f"got '{lp.type}'"
)

# Pin RNG seeds from the submission ruleset. Done last so the values
# are baked into the config before any consumer reads them — the config
# dump to the report dir, RuntimeSettings.from_config, and the report
# seeds block all see the pinned values.
self._apply_ruleset_seed_overrides()

return self

def _apply_ruleset_seed_overrides(self) -> None:
"""Override runtime RNG seeds from the selected submission ruleset.

MLPerf rounds pin the RNG seeds; this mirrors LoadGen locking the core
seeds from ``user.conf`` (a submitter cannot substitute their own).
If ``submission_ref`` is unset, the config is left unchanged. If it
names an unregistered ruleset, a ``type=SUBMISSION`` config errors (a
submission cannot silently fall back to default seeds), while any other
type is left unchanged so non-submission/placeholder configs still work.
"""
if self.submission_ref is None:
return
try:
ruleset = self.submission_ref.get_ruleset_instance()
except KeyError as e:
if self.type == TestType.SUBMISSION:

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

[Claude] medium / api-contract: Breaking change for existing v6.0 submission configs. This branch raises ValidationError at config-load when a type=submission config names an unregistered ruleset. The prior submission_template.yaml on main shipped ruleset: "mlperf-inference-v6.0", which is not registered (only v5.1 / v6.1 / mlcommons-current are). Any user config copied from the old template now hard-fails to load instead of running. The behavior is correct in principle (a submission must pin official seeds), but it warrants a changelog / migration note for in-flight v6.0 configs.

raise ValueError(
f"submission_ref.ruleset {self.submission_ref.ruleset!r} is not "
"registered; a submission must pin official RNG seeds and cannot "
"fall back to defaults."
) from e
logger.warning(
"submission_ref.ruleset %r is not registered; skipping ruleset "
"seed overrides.",
self.submission_ref.ruleset,
)
return

updates: dict[str, int] = {}
if ruleset.scheduler_rng_seed is not None:

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

[Claude] medium (testing): The two is not None seed branches and the if not updates: return early-exit are never exercised — both registered MLCommons rulesets set both seeds. ruleset_base.py documents None as a valid "unseeded randomization" value, so a partial ruleset (one seed None) or a no-op ruleset (both None) is a supported, reachable state. Add a test that registers a throwaway RoundRuleset(scheduler_rng_seed=None, sample_index_rng_seed=<int>) (and one with both None) and asserts only the set seed is pinned / the config is left unchanged.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There is a semantic issue beyond the branch coverage discussed above: None currently means two different things. BenchmarkSuiteRuleset documents it as unseeded randomization, and RoundRuleset.apply_user_config passes it through to random.Random(None); this config path instead skips the override and retains the default/user seed, usually 42. The same ruleset therefore behaves differently depending on its construction path.

Suggested fix: preserve the existing contract by making the runtime/report seed fields nullable and always applying both ruleset values, including None:

updates = {
    "scheduler_random_seed": ruleset.scheduler_rng_seed,
    "dataloader_random_seed": ruleset.sample_index_rng_seed,
}

If nullable seeds are not intended, the alternative is to remove None from the ruleset contract and reject such rulesets explicitly. Either way, the two runtime-construction paths should agree.

updates["scheduler_random_seed"] = ruleset.scheduler_rng_seed
if ruleset.sample_index_rng_seed is not None:
updates["dataloader_random_seed"] = ruleset.sample_index_rng_seed
if not updates:
return

# model_copy(update=) writes straight into __dict__ without validating
# that the keys are real fields; guard so a future RuntimeConfig rename
# fails loudly instead of silently attaching a junk attribute.
runtime_fields = type(self.settings.runtime).model_fields
unknown = updates.keys() - runtime_fields.keys()
if unknown:
raise ValueError(f"seed override targets unknown runtime fields: {unknown}")
new_runtime = self.settings.runtime.model_copy(update=updates)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

[Claude] low (error-handling): self.settings.runtime.model_copy(update=updates) — pydantic v2 model_copy(update=...) bypasses validation and does not verify that update keys are real fields (it writes straight into __dict__, even with extra="forbid"). The keys "scheduler_random_seed"/"dataloader_random_seed" are hardcoded strings decoupled from the RuntimeConfig field definitions; if either field is renamed, this silently attaches a junk attribute and leaves the real seed at its default with no error. Consider deriving the names from the model (or a shared constant) so a rename fails loudly.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Follow-up after the field-name guard: model_copy(update=...) still bypasses value validation. I registered a ruleset with scheduler_rng_seed="oops" and obtained a successfully validated BenchmarkConfig whose runtime seed was a str.

Suggested fix: rebuild through Pydantic validation instead of an unvalidated copy:

new_runtime = type(self.settings.runtime).model_validate(
    {**self.settings.runtime.model_dump(), **updates}
)

With extra="forbid", this also catches unknown fields, making the manual key guard redundant while validating seed values. Please add an invalid-seed regression test.

object.__setattr__(
self,
"settings",
self.settings.model_copy(update={"runtime": new_runtime}),
)
logger.debug(
"Pinned RNG seeds from ruleset %r: %s", self.submission_ref.ruleset, updates
)

@model_validator(mode="after")
def _propagate_client_api_type(self) -> Self:
"""Sync client.api_type from endpoint_config.api_type at construction.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ benchmark_mode: "offline" # Required for submission: "offline" or "online"
# Submission reference (model + ruleset for compliance checks)
submission_ref:
model: "llama-2-70b"
ruleset: "mlperf-inference-v6.0"
ruleset: "mlperf-inference-v6.1"

model_params:
name: "meta-llama/Llama-2-70B-Chat-HF"
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/commands/test_benchmark.py
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ def test_from_config_submission_defaults_to_both(self, mock_run, tmp_path):
- path: "test.jsonl"
submission_ref:
model: "test-model"
ruleset: "test"
ruleset: "mlperf-inference-v6.1"
"""
config_file = tmp_path / "sub.yaml"
config_file.write_text(yaml_content)
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/commands/test_util_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,12 @@ def test_submission_ref_logging(self, tmp_path, caplog):
- path: "test.jsonl"
submission_ref:
model: "llama"
ruleset: "mlperf-v6"
ruleset: "mlperf-inference-v6.1"
""")
with caplog.at_level("INFO"):
execute_validate(config_file)
assert "Submission:" in caplog.text
assert "mlperf-v6" in caplog.text
assert "mlperf-inference-v6.1" in caplog.text

@pytest.mark.unit
def test_invalid_yaml(self, tmp_path):
Expand Down
63 changes: 62 additions & 1 deletion tests/unit/config/rulesets/mlcommons/test_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,14 @@

import pytest
from inference_endpoint import metrics
from inference_endpoint.config.ruleset_registry import get_ruleset, list_rulesets
from inference_endpoint.config.ruleset_registry import (
get_ruleset,
list_rulesets,
register_ruleset,
)
from inference_endpoint.config.rulesets.mlcommons import datasets, models
from inference_endpoint.config.rulesets.mlcommons.rules import (
ALL_ROUNDS,
CURRENT,
EDGE_CURRENT,
OptimizationPriority,
Expand Down Expand Up @@ -116,6 +121,62 @@ def test_apply_user_config_min_sample_count_override():


@pytest.mark.unit
def test_current_round_is_v6_1():
assert CURRENT.version == "v6.1"


@pytest.mark.unit
def test_v6_1_official_seeds():
"""Seeds are the schedule/sample_index values from loadgen/mlperf.conf."""
v6_1 = get_ruleset("mlperf-inference-v6.1")
assert v6_1.scheduler_rng_seed == 16159082839903944936
assert v6_1.sample_index_rng_seed == 2747215439041700203


@pytest.mark.unit
def test_all_rounds_registered_by_version():
names = list_rulesets()
for ruleset in ALL_ROUNDS:
assert f"mlperf-inference-{ruleset.version}" in names
# Both the prior and current round remain resolvable.
assert "mlperf-inference-v5.1" in names
assert "mlperf-inference-v6.1" in names
assert get_ruleset("mlcommons-current") is CURRENT


@pytest.mark.unit
def test_round_versions_are_unique():
"""Each registered round must own a distinct version string, else the
registry would silently overwrite one round's entry with another's."""
assert len(ALL_ROUNDS) == len({r.version for r in ALL_ROUNDS})


@pytest.mark.unit
def test_register_ruleset_rejects_duplicate():
"""Re-registering an existing name raises rather than clobbering it."""
with pytest.raises(ValueError):
register_ruleset("mlperf-inference-v6.1", CURRENT)


@pytest.mark.unit
def test_v6_1_latency_targets_match_v5_1():
"""Only the round seeds rotate; per-model targets are identical."""
v5_1 = get_ruleset("mlperf-inference-v5.1")
v6_1 = get_ruleset("mlperf-inference-v6.1")
assert v6_1.benchmark_rulesets == v5_1.benchmark_rulesets

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

[Claude] low (testing): v6_1.benchmark_rulesets == v5_1.benchmark_rulesets is identity-trivially true — _v6_1 is built with benchmark_rulesets=_v5_1.benchmark_rulesets (the same dict object), so this passes without actually verifying the per-model targets. (If the shared reference is replaced by a copy — see the existing suggestion on rules.py:282 — this becomes a real structural check.) Consider asserting a concrete expected value, e.g. a specific model's max_tpot_latency_ms, instead.

# v6.1 holds an independent copy, not a shared reference to v5.1's dict.
assert v6_1.benchmark_rulesets is not v5_1.benchmark_rulesets
# Anchor on a concrete target so the equality above verifies real structure.
assert (
v6_1.benchmark_rulesets[models.Llama3_1_405b][
OptimizationPriority.THROUGHPUT
].max_tpot_latency_ms
== 175
)
assert v6_1.scheduler_rng_seed != v5_1.scheduler_rng_seed
assert v6_1.sample_index_rng_seed != v5_1.sample_index_rng_seed


def test_edge_ruleset_registered():
# Resolvable by version-specific name and the "current" alias.
assert get_ruleset("mlperf-edge-v0.1") is EDGE_CURRENT
Expand Down
Loading
Loading