Skip to content

Commit 9d08097

Browse files
committed
cleanup: remove dead container_image/digest/name fields from ModelConfig
- Removed name, revision, container_image, container_digest from ModelConfig (all moved to identity block) - Removed Docker image pre-submit validation (can't verify from inside Pyxis) - HF validation now reads from identity.model.repo - Updated tests to use IdentityConfig instead of old ModelConfig fields - Note: background validation thread was effectively dead code — daemon thread exits before HTTP completes. Left in place but it needs a rethink.
1 parent d97ed25 commit 9d08097

5 files changed

Lines changed: 36 additions & 66 deletions

File tree

recipes/mocker/kimi-trace-agg.yaml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,7 @@ slurm:
55

66
model:
77
path: "kimi-k25-nvfp4"
8-
name: "nvidia/Kimi-K2.5-NVFP4" # HuggingFace model ID (virtual identity for reproducibility)
98
container: "trtllm-runtime"
10-
container_image: "gitlab-master.nvidia.com:5005/dl/ai-dynamo/dynamo:56f877977-trtllm-arm64" # pullable Docker URI
119
precision: "fp4"
1210

1311
resources:

src/srtctl/core/schema.py

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -385,12 +385,6 @@ class ModelConfig:
385385
container: str
386386
precision: str
387387

388-
# Virtual identity (optional, for reproducibility and lockfile)
389-
name: str | None = None # HuggingFace model ID, e.g. "deepseek-ai/DeepSeek-R1"
390-
revision: str | None = None # HuggingFace git commit SHA
391-
container_image: str | None = None # Docker image tag, e.g. "lmsysorg/sglang:v0.4.6"
392-
container_digest: str | None = None # Docker manifest digest, e.g. "sha256:abc..."
393-
394388
Schema: ClassVar[type[Schema]] = Schema
395389

396390

src/srtctl/core/validation.py

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -143,29 +143,17 @@ def run_all_validations(config: SrtConfig) -> list[ValidationResult]:
143143
except Exception as e:
144144
results.append(ValidationResult("container_path", False, f"check failed: {e}"))
145145

146-
# HuggingFace model — check identity block first, fall back to model config
146+
# HuggingFace model (from identity block)
147147
try:
148148
hf_repo = None
149149
hf_rev = None
150150
if hasattr(config, "identity") and config.identity and config.identity.model:
151151
hf_repo = config.identity.model.repo
152152
hf_rev = config.identity.model.revision
153-
if not hf_repo and hasattr(config.model, "name"):
154-
hf_repo = config.model.name
155-
if not hf_rev and hasattr(config.model, "revision"):
156-
hf_rev = config.model.revision
157153
results.append(validate_hf_model(hf_repo, hf_rev))
158154
except Exception as e:
159155
results.append(ValidationResult("hf_model", False, f"check failed: {e}"))
160156

161-
# Docker image (if set on model config)
162-
try:
163-
container_image = getattr(config.model, "container_image", None)
164-
container_digest = getattr(config.model, "container_digest", None)
165-
results.append(validate_docker_image(container_image, container_digest))
166-
except Exception as e:
167-
results.append(ValidationResult("docker_image", False, f"check failed: {e}"))
168-
169157
return results
170158

171159

tests/test_configs.py

Lines changed: 32 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -93,63 +93,53 @@ def test_decode_nodes_zero_inherits_tp_from_prefill(self):
9393
assert total_needed <= total_available
9494

9595

96-
class TestModelConfigIdentity:
97-
"""Tests for optional virtual identity fields on ModelConfig."""
96+
class TestIdentityConfig:
97+
"""Tests for the identity block (virtual identity for runtime verification)."""
9898

99-
def test_optional_fields_default_to_none(self):
100-
"""All identity fields are None when not specified."""
101-
from srtctl.core.schema import ModelConfig
99+
def test_defaults_to_empty(self):
100+
"""IdentityConfig has empty defaults."""
101+
from srtctl.core.schema import IdentityConfig
102102

103-
config = ModelConfig(path="/model", container="/c.sqsh", precision="fp8")
104-
assert config.name is None
105-
assert config.revision is None
106-
assert config.container_image is None
107-
assert config.container_digest is None
103+
config = IdentityConfig()
104+
assert config.model.repo is None
105+
assert config.model.revision is None
106+
assert config.frameworks == {}
108107

109-
def test_optional_fields_when_specified(self):
110-
"""Identity fields are set when provided."""
111-
from srtctl.core.schema import ModelConfig
108+
def test_with_values(self):
109+
"""IdentityConfig stores model and framework info."""
110+
from srtctl.core.schema import IdentityConfig, IdentityModelConfig
112111

113-
config = ModelConfig(
114-
path="/model",
115-
container="/c.sqsh",
116-
precision="fp4",
117-
name="deepseek-ai/DeepSeek-R1",
118-
revision="e4e908c07378",
119-
container_image="lmsysorg/sglang:v0.4.6.post1",
120-
container_digest="sha256:abc123",
112+
config = IdentityConfig(
113+
model=IdentityModelConfig(repo="nvidia/Kimi-K2.5-NVFP4", revision="abc123"),
114+
frameworks={"dynamo": "1.0.0", "tensorrt_llm": "1.3.0rc9"},
121115
)
122-
assert config.name == "deepseek-ai/DeepSeek-R1"
123-
assert config.revision == "e4e908c07378"
124-
assert config.container_image == "lmsysorg/sglang:v0.4.6.post1"
125-
assert config.container_digest == "sha256:abc123"
116+
assert config.model.repo == "nvidia/Kimi-K2.5-NVFP4"
117+
assert config.model.revision == "abc123"
118+
assert config.frameworks["dynamo"] == "1.0.0"
119+
assert config.frameworks["tensorrt_llm"] == "1.3.0rc9"
126120

127121
def test_marshmallow_roundtrip(self):
128122
"""Schema dump/load preserves identity fields."""
129-
from srtctl.core.schema import ModelConfig
123+
from srtctl.core.schema import IdentityConfig, IdentityModelConfig
130124

131-
original = ModelConfig(
132-
path="/model",
133-
container="/c.sqsh",
134-
precision="fp4",
135-
name="deepseek-ai/DeepSeek-R1",
136-
revision="abc123",
125+
original = IdentityConfig(
126+
model=IdentityModelConfig(repo="nvidia/Kimi-K2.5-NVFP4", revision="abc123"),
127+
frameworks={"dynamo": "1.0.0"},
137128
)
138-
schema = ModelConfig.Schema()
129+
schema = IdentityConfig.Schema()
139130
dumped = schema.dump(original)
140131
loaded = schema.load(dumped)
141-
assert loaded.name == "deepseek-ai/DeepSeek-R1"
142-
assert loaded.revision == "abc123"
143-
assert loaded.container_image is None # was not set
132+
assert loaded.model.repo == "nvidia/Kimi-K2.5-NVFP4"
133+
assert loaded.frameworks["dynamo"] == "1.0.0"
144134

145-
def test_marshmallow_load_ignores_missing_optional(self):
146-
"""Loading YAML without identity fields still works."""
135+
def test_model_config_is_clean(self):
136+
"""ModelConfig has no virtual identity fields (moved to IdentityConfig)."""
147137
from srtctl.core.schema import ModelConfig
148138

149-
data = {"path": "/model", "container": "/c.sqsh", "precision": "fp8"}
150-
loaded = ModelConfig.Schema().load(data)
151-
assert loaded.name is None
152-
assert loaded.revision is None
139+
config = ModelConfig(path="/model", container="/c.sqsh", precision="fp8")
140+
assert not hasattr(config, "name")
141+
assert not hasattr(config, "container_image")
142+
assert not hasattr(config, "container_digest")
153143

154144

155145
class TestDynamoConfig:

tests/test_validation.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -200,10 +200,11 @@ def test_all_checks_produce_results(self):
200200
"path": "/nonexistent",
201201
"container": "/nonexistent.sqsh",
202202
"precision": "fp8",
203-
"name": "some/model",
204-
"container_image": "some/image:tag",
205203
},
206204
"resources": {"gpu_type": "h100", "gpus_per_node": 8, "prefill_nodes": 1, "decode_nodes": 1},
205+
"identity": {
206+
"model": {"repo": "some/model"},
207+
},
207208
})
208209

209210
with patch("srtctl.core.validation.requests.head", side_effect=requests.ConnectionError()):
@@ -213,7 +214,6 @@ def test_all_checks_produce_results(self):
213214
assert "model_path" in check_names
214215
assert "container_path" in check_names
215216
assert "hf_model" in check_names
216-
assert "docker_image" in check_names
217217

218218

219219
# ============================================================================

0 commit comments

Comments
 (0)