Conversation
|
|
||
| if isinstance(default_server_configs, APIServerConfig): | ||
| server_configs = final_openai_config | ||
| server_configs = [final_openai_config] |
There was a problem hiding this comment.
If you pass a list of configs here it uses the configs directly. But if you pass a single non list config object, it goes into "template mode" and auto-generates server URLs/ports
There was a problem hiding this comment.
I mean, you're not supposed to pass this in like that
There was a problem hiding this comment.
Agreed — the issue was the wrong config shape here. I fixed it so this path now returns [final_openai_config] instead of a bare APIServerConfig.
dmahan93
left a comment
dmahan93
left a comment
There was a problem hiding this comment.
i do not think different tokenizer distillation is possible so at minimum remove it from this pr, if it is possible move to dms and we can get a blog post out
| teacher_base_url: Optional[str] = Field( | ||
| default=None, | ||
| description="Teacher server base URL (OpenAI-compatible).", | ||
| ) | ||
| teacher_model_name: Optional[str] = Field( | ||
| default=None, | ||
| description="Teacher model name used in teacher server requests.", | ||
| ) | ||
| teacher_api_key: str = Field( | ||
| default="", | ||
| description="Teacher API key, if required by the teacher endpoint.", | ||
| ) | ||
| teacher_server_type: str = Field( | ||
| default="vllm", | ||
| description="Teacher server type (e.g. vllm, sglang, trl, openai).", | ||
| ) | ||
| teacher_tokenizer_name: str = Field( | ||
| default="none", | ||
| description=( | ||
| "Tokenizer name for teacher server. If 'none', teacher_model_name is used. " | ||
| "When this resolves to a different vocabulary than the student tokenizer, " | ||
| "cross-tokenizer alignment is applied automatically." | ||
| ), | ||
| ) |
There was a problem hiding this comment.
can't we just make these ServerConfigs
There was a problem hiding this comment.
Yes — I moved those endpoint fields into teacher_server: APIServerConfig on TeacherDistillationConfig
environments/gsm8k_server.py
Outdated
| ) | ||
|
|
||
| async with self.server.managed_server(tokenizer=self.tokenizer) as managed: | ||
| logger.warning( |
environments/gsm8k_server.py
Outdated
| max_tokens=self.config.max_token_length, | ||
| temperature=1.0, | ||
| ) | ||
| logger.warning( |
environments/gsm8k_server.py
Outdated
|
|
||
| state = managed.get_state() | ||
| nodes = state["nodes"] | ||
| logger.warning( |
example_trainer/api.py
Outdated
| "X-Atropos-Client": "trainer", | ||
| "X-Atropos-Pid": str(os.getpid()), |
There was a problem hiding this comment.
This was a sanity check - it has been removed
|
|
||
| if isinstance(default_server_configs, APIServerConfig): | ||
| server_configs = final_openai_config | ||
| server_configs = [final_openai_config] |
There was a problem hiding this comment.
I mean, you're not supposed to pass this in like that
| teacher_server: Optional[APIServerConfig] = Field( | ||
| default=None, | ||
| description="Teacher inference server configuration.", | ||
| ) |
There was a problem hiding this comment.
ah, i probably commented poorly, it should be the same as how we setup the server_manager, so we may need to pass in a new thing to init
There was a problem hiding this comment.
Updated this to follow this pattern. I removed teacher_server from TeacherDistillationConfig, so the env config now only carries env-level knobs like teacher_enabled and teacher_top_k. Teacher server wiring is now passed separately via teacher_server_configs at init
| "teacher_top_k", self.config.teacher_top_k | ||
| ) | ||
| ) | ||
| top_k = max(1, top_k) |
There was a problem hiding this comment.
max should be 0, because prompt logprobs are (selected token + topk), disabled would be setting it to -1 or lower. I would also be amenable to a group override that's skip_teacher_top_k
There was a problem hiding this comment.
i think this may need to be reverted?
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
2 participants
No description provided.