-
Notifications
You must be signed in to change notification settings - Fork 400
Initialize base inference before training model creation #2003
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -429,6 +429,18 @@ def _ensure_inference_engines(self): | |||||||||||
| self._render_server.shutdown() | ||||||||||||
| self._render_server = None | ||||||||||||
|
|
||||||||||||
| def initialize_base_inference(self) -> None: | ||||||||||||
| """Start base-model inference before the first training model is created.""" | ||||||||||||
| if self._inference_engines_initialized: | ||||||||||||
| return | ||||||||||||
| self._cfg = _build_skyrl_train_config(self.base_model, self.config) | ||||||||||||
| if not ray.is_initialized(): | ||||||||||||
| logger.info("Initializing Ray for base-model inference") | ||||||||||||
| initialize_ray(self._cfg) | ||||||||||||
| self._colocate_pg = self._create_colocate_pg() if self._cfg.trainer.placement.colocate_all else None | ||||||||||||
| self._create_new_inference_client() | ||||||||||||
| self._inference_engines_initialized = True | ||||||||||||
|
|
||||||||||||
| def _lora_signature_from(self, lora_config: types.LoraConfig) -> tuple: | ||||||||||||
| # Tinker's public LoraConfig only exposes rank + alpha (plus | ||||||||||||
| # seed/train_attn/train_mlp/train_unembed) - pending support https://github.com/NovaSky-AI/SkyRL/issues/1632. | ||||||||||||
|
|
@@ -497,6 +509,9 @@ def create_model(self, model_id: str, lora_config: types.LoraConfig, model_role: | |||||||||||
|
|
||||||||||||
| logger.info("Building models.") | ||||||||||||
| self._build_policy(PolicyWorker, model_id=model_id) | ||||||||||||
| if self._inference_engines_initialized: | ||||||||||||
| self._dispatch.set_inference_engine_client(self._inference_engine_client) | ||||||||||||
| self.init_weight_sync_state() | ||||||||||||
|
Comment on lines
+512
to
+514
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The call to
Suggested change
|
||||||||||||
| if is_lora: | ||||||||||||
| self._base_lora_signature = self._lora_signature_from(lora_config) | ||||||||||||
| elif model_role == "critic": | ||||||||||||
|
|
||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -272,6 +272,10 @@ def __init__( | |
| if hasattr(self.backend, "set_inference_state_publisher"): | ||
| self.backend.set_inference_state_publisher(self._write_inference_state_to_db) | ||
|
|
||
| is_colocated = bool(config.backend_config.get("trainer.placement.colocate_all", True)) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using colocate_val = config.backend_config.get("trainer.placement.colocate_all", True)
is_colocated = colocate_val not in (False, "False", "false", 0, "0") |
||
| if not is_colocated and hasattr(self.backend, "initialize_base_inference"): | ||
| self.backend.initialize_base_inference() | ||
|
|
||
| # Track last cleanup time for periodic stale session cleanup | ||
| self._last_cleanup_time: float = time.time() | ||
|
|
||
|
|
@@ -295,6 +299,11 @@ def _write_inference_state_to_db(self, proxy_url: str | None) -> None: | |
| row.updated_at = datetime.now(timezone.utc) | ||
| session.add(row) | ||
| session.commit() | ||
| if proxy_url is not None: | ||
| logger.info( | ||
| 'SKYRL_DEPLOYMENT_EVENT {"event":"inference_proxy_published","model":"%s"}', | ||
| self.config.base_model, | ||
| ) | ||
|
|
||
| @contextmanager | ||
| def _checkpoint_status_context(self, model_id: str, checkpoint_id: str, checkpoint_type: types.CheckpointType): | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like
initialize_base_inference()is only ever called in non-colocated mode. What is the purpose of having this check here?