Skip to content

Commit 79bd657

Browse files
avigyabbclaude
andcommitted
[tinker] Support tinker SDK 0.32/0.33 (pin <0.34)
Widen the SDK pin to >=0.25.0,<0.34.0 and lock 0.33.1. - proto_serialization: 0.28 renamed TopkPromptLogprobs -> TopkLogprobs (prompt_length -> length, same field numbers). Pick whichever the installed SDK schema has, like loss_fn_config_v2. - JAX backend: emit loss:sum (sum of per-token losses) from forward_backward; SDK >= 0.31 custom-loss pops it. The skyrl-train backend got the same alias in #2455. - API: accept 0.32's optimizer_config on create_model/load_weights and optimizer_params on optim_step, rejecting anything but Adam with a 422. Reject the new SampleRequest options the engines cannot produce (topk_sample_logprobs, target_prompt_logprobs, prompt_alt_tokens_k, 0.33's prompt_logprobs_last_n) instead of returning None for them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gg8qLdfM4FSHLBjyQHZX2y Signed-off-by: Avi Basnet <avigya.basnet@anyscale.com>
1 parent 3d0ae22 commit 79bd657

8 files changed

Lines changed: 159 additions & 20 deletions

File tree

‎.agents/docs/tinker.md‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,15 @@ All endpoints are under `/api/v1/`. Requests are async -- submit via POST, get a
4444

4545
## SDK compatibility
4646

47-
`pyproject.toml` pins `tinker>=0.25.0,<0.28.0`; `uv.lock` resolves 0.27.2. Verified against 0.25.0, 0.27.0 and 0.27.2. What each SDK line needs from the server:
47+
`pyproject.toml` pins `tinker>=0.25.0,<0.34.0`; `uv.lock` resolves 0.33.1. The server encodes/decodes with whichever SDK's `tinker_public_pb2` is installed, so `proto_serialization.py` feature-detects schema changes instead of assuming one line. What each SDK line needs from the server:
4848

4949
- **0.25**: proto `forward_backward` bodies and proto `retrieve_future` results (`proto_serialization.py`).
50-
- **0.26/0.27**: `asample` must return `sample_sequence_ids`, one per `num_samples` (`SampleFutureResponse`); the SDK stamps them onto `SampledSequence.sequence_id` client-side, so the proto result does not carry them. `forward_backward` requests dual-write `loss_fn_config` into a legacy float map and `loss_fn_config_v2` (number/text union); the parser prefers v2 so string values reach the API model (which rejects them, since no SkyRL loss takes a string).
51-
- **0.28 is not supported yet**: it renames the proto `TopkPromptLogprobs` message to `TopkLogprobs` (`prompt_length` -> `length`), which breaks `serialize_sample_output` for `topk_prompt_logprobs > 0`, and adds `save_weights_external`.
50+
- **0.26/0.27**: `asample` must return `sample_sequence_ids`, one per `num_samples` (`SampleFutureResponse`); the SDK stamps them onto `SampledSequence.sequence_id` client-side. `forward_backward` requests dual-write `loss_fn_config` into a legacy float map and `loss_fn_config_v2` (number/text union); the parser prefers v2 so string values reach the API model (which rejects them, since no SkyRL loss takes a string).
51+
- **0.28**: proto `TopkPromptLogprobs` renamed to `TopkLogprobs` (`prompt_length` -> `length`); the top-k serializer picks whichever the installed SDK has.
52+
- **0.31+**: `forward_backward_custom` pops `loss:sum` from the result metrics (the Tinker service's name for the training loss). Both backends emit it: `skyrl_train_backend._extract_metrics` mirrors `total_loss:sum`, the JAX backend sums the per-token losses.
53+
- **0.32**: optimizer families. `create_model`/`load_weights` carry `optimizer_config` and `optim_step` sends non-Adam params under `optimizer_params` instead of `adam_params`; anything but Adam is a 422. New `SampleRequest` options the engines do not produce (`topk_sample_logprobs`, `target_prompt_logprobs`, `prompt_alt_tokens_k`, and 0.33's `prompt_logprobs_last_n`) are rejected rather than silently ignored.
5254

53-
0.27 client features that are opt-in behind server-driven flags and that SkyRL leaves off (the stub `/api/v1/client/config` returns defaults): `sample_use_retrieve_futures` (`/api/v1/retrieve_futures` session poller), `sample_cancel_enabled` (`/api/v1/cancel_future`). Not implemented and ignored: `copy_weights`, checkpoint `user_metadata`, `Datum` provenance spans, `SampleRequest.record_stability_info`.
55+
Client features that are opt-in behind server-driven flags and that SkyRL leaves off (the stub `/api/v1/client/config` returns defaults): `sample_use_retrieve_futures` (`/api/v1/retrieve_futures` session poller), `sample_join_sampling_session` (`/api/v1/join_sampling_session`), dynamic-config `sample_cancel_enabled` (`/api/v1/cancel_future`). Not implemented: `copy_weights`, `save_weights_external` and external weights URLs, checkpoint `user_metadata`, `Datum` provenance spans, `SampleRequest.record_stability_info`, the checkpoint billing route.
5456

5557
## Concurrency and Batching
5658

‎pyproject.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ tpu = [
3939
]
4040

4141
tinker = [
42-
"tinker>=0.25.0,<0.28.0",
42+
"tinker>=0.25.0,<0.34.0",
4343
"fastapi[standard]",
4444
"sqlmodel",
4545
"sqlalchemy[asyncio]",

‎skyrl/backends/jax.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -757,11 +757,13 @@ def _model_pass(
757757
# Compute per-request results
758758
for request_id, _, start_idx, end_idx in request_batch_slices:
759759
loss_fn_outputs = []
760+
loss_sum = 0.0
760761
# Compute per-example losses
761762
for i in range(start_idx, end_idx):
762763
# Extract losses for this example's tokens
763764
token_losses = token_losses_out[i]
764765
token_logprobs = logprobs_out[i]
766+
loss_sum += float(token_losses.sum())
765767
loss_fn_outputs.append(
766768
{
767769
"elementwise_loss": {
@@ -780,7 +782,9 @@ def _model_pass(
780782
results[request_id] = types.ForwardBackwardOutput(
781783
loss_fn_output_type="scalar",
782784
loss_fn_outputs=loss_fn_outputs,
783-
metrics={},
785+
# The SDK's custom-loss path (>= 0.31) pops "loss:sum" from the
786+
# result to report the surrogate loss.
787+
metrics={"loss:sum": loss_sum},
784788
)
785789

786790
return results

‎skyrl/tinker/api.py‎

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
Field,
2626
Tag,
2727
ValidationError,
28+
field_validator,
2829
model_validator,
2930
)
3031
from sqlalchemy.exc import IntegrityError
@@ -531,11 +532,23 @@ class LoRAConfig(BaseModel):
531532
)
532533

533534

535+
def _validate_adam_optimizer_config(optimizer_config: dict[str, Any] | None) -> dict[str, Any] | None:
536+
"""SDK >= 0.32 selects the optimizer family at model creation; SkyRL only has Adam."""
537+
if optimizer_config is not None and optimizer_config.get("type", "adamw") != "adamw":
538+
raise ValueError(
539+
f"Unsupported optimizer_config type {optimizer_config.get('type')!r}; only 'adamw' is supported"
540+
)
541+
return optimizer_config
542+
543+
534544
class CreateModelRequest(BaseModel):
535545
session_id: str
536546
base_model: str
537547
lora_config: LoRAConfig
538548
model_role: str = "policy"
549+
optimizer_config: dict[str, Any] | None = None
550+
551+
_check_optimizer = field_validator("optimizer_config")(_validate_adam_optimizer_config)
539552

540553

541554
class CreateModelResponse(BaseModel):
@@ -762,9 +775,21 @@ def to_types(self) -> types.AdamParams:
762775

763776
class OptimStepRequest(BaseModel):
764777
model_id: str
765-
adam_params: AdamParams
778+
adam_params: AdamParams | None = None
779+
# SDK >= 0.32 sends non-Adam families (e.g. Dimuon) under this key instead.
780+
optimizer_params: dict[str, Any] | None = None
766781
seq_id: int | None = None
767782

783+
@model_validator(mode="after")
784+
def validate_adam_only(self):
785+
if self.optimizer_params is not None:
786+
raise ValueError(
787+
f"Unsupported optimizer_params type {self.optimizer_params.get('type')!r}; only Adam (adam_params) is supported"
788+
)
789+
if self.adam_params is None:
790+
raise ValueError("adam_params is required")
791+
return self
792+
768793

769794
class SaveWeightsForSamplerRequest(BaseModel):
770795
model_id: str
@@ -832,8 +857,31 @@ class SampleRequest(BaseModel):
832857
seq_id: int | None = None
833858
prompt_logprobs: bool | None = None
834859
topk_prompt_logprobs: int = Field(default=0, ge=0)
860+
# SDK >= 0.32 / 0.33 request options SkyRL's engines do not produce. They
861+
# are rejected rather than ignored, since the SDK would otherwise hand the
862+
# caller None for data it asked for.
863+
topk_sample_logprobs: int = 0
864+
target_prompt_logprobs: Any | None = None
865+
prompt_alt_tokens_k: int = 0
866+
prompt_logprobs_last_n: int | None = None
835867
type: Literal["sample"] = "sample"
836868

869+
@model_validator(mode="after")
870+
def validate_unsupported_options(self):
871+
unsupported = [
872+
name
873+
for name, value in (
874+
("topk_sample_logprobs", self.topk_sample_logprobs),
875+
("target_prompt_logprobs", self.target_prompt_logprobs),
876+
("prompt_alt_tokens_k", self.prompt_alt_tokens_k),
877+
("prompt_logprobs_last_n", self.prompt_logprobs_last_n),
878+
)
879+
if value not in (0, None)
880+
]
881+
if unsupported:
882+
raise ValueError(f"Unsupported sampling options: {unsupported}")
883+
return self
884+
837885
@model_validator(mode="after")
838886
def validate_model_source(self):
839887
"""Valid if:
@@ -862,8 +910,11 @@ class LoadWeightsRequest(BaseModel):
862910
path: str
863911
optimizer: bool = True
864912
seq_id: int | None = None
913+
optimizer_config: dict[str, Any] | None = None
865914
type: Literal["load_weights"] | None = None
866915

916+
_check_optimizer = field_validator("optimizer_config")(_validate_adam_optimizer_config)
917+
867918

868919
class FutureResponse(BaseModel):
869920
future_id: str
@@ -1498,7 +1549,7 @@ async def optim_step(request: OptimStepRequest, session: AsyncSession = Depends(
14981549
session=session,
14991550
request_type=types.RequestType.OPTIM_STEP,
15001551
model_id=request.model_id,
1501-
request_data=types.OptimStepInput(adam_params=request.adam_params.to_types()),
1552+
request_data=types.OptimStepInput(adam_params=request.adam_params.to_types()), # validated non-None
15021553
seq_id=request.seq_id,
15031554
)
15041555

‎skyrl/tinker/proto_serialization.py‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
"""Proto wire-format conversion for the Tinker SDK's binary paths.
22
33
Two directions, both required by tinker SDK >= 0.25.0 (older SDKs negotiate
4-
these via server-side client_config flags and fall back to JSON). Written
5-
against the SDK's ``tinker_public.proto`` as of 0.27.x; 0.28.0 renamed
6-
``TopkPromptLogprobs`` to ``TopkLogprobs`` (``prompt_length`` -> ``length``),
7-
which the top-k serializer below does not yet handle:
4+
these via server-side client_config flags and fall back to JSON). The server
5+
encodes with whichever SDK's ``tinker_public_pb2`` is installed, so schema
6+
changes across SDK lines (``loss_fn_config_v2`` in 0.27, the
7+
``TopkPromptLogprobs`` -> ``TopkLogprobs`` rename in 0.28) are feature-detected
8+
below rather than assumed:
89
910
- **Responses**: the SDK retrieves ``sample``, ``forward``, and
1011
``forward_backward`` results as protobuf (``Accept: application/x-protobuf``
@@ -65,6 +66,11 @@
6566
# against whichever SDK is installed, so feature-detect it.
6667
_HAS_LOSS_FN_CONFIG_V2 = "loss_fn_config_v2" in pb.ForwardBackwardRequest.DESCRIPTOR.fields_by_name
6768

69+
# SDK >= 0.28 renamed the top-k matrix message (and its row-count field) when
70+
# it started reusing it for per-sequence sampled top-k.
71+
_TOPK_MESSAGE = getattr(pb, "TopkLogprobs", None) or pb.TopkPromptLogprobs
72+
_TOPK_LENGTH_FIELD = "length" if "length" in _TOPK_MESSAGE.DESCRIPTOR.fields_by_name else "prompt_length"
73+
6874

6975
def parse_forward_backward_request(body: bytes) -> tuple[dict, bool]:
7076
"""Parse a proto ForwardBackwardRequest body into the JSON-equivalent
@@ -194,7 +200,7 @@ def serialize_sample_output(
194200
if topk_prompt_logprobs is not None:
195201
rows = topk_prompt_logprobs
196202
# k is not recorded in the result, so recover it from the widest row.
197-
# With every row undefined, use k=1 so prompt_length stays encoded
203+
# With every row undefined, use k=1 so the row count stays encoded
198204
# (the client maps fully-masked rows back to None).
199205
k = max((len(row) for row in rows if row), default=1)
200206
token_ids = np.full((len(rows), k), _TOPK_MASK_TOKEN_ID, dtype=np.int32)
@@ -204,11 +210,11 @@ def serialize_sample_output(
204210
token_ids[i, j] = token_id
205211
logprobs[i, j] = logprob
206212
proto.topk_prompt_logprobs.CopyFrom(
207-
pb.TopkPromptLogprobs(
208-
prompt_length=len(rows),
213+
_TOPK_MESSAGE(
209214
k=k,
210215
token_ids=token_ids.tobytes(),
211216
logprobs=logprobs.tobytes(),
217+
**{_TOPK_LENGTH_FIELD: len(rows)},
212218
)
213219
)
214220

‎tests/backends/test_jax_backend.py‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,13 @@ def test_micro_batch_grad_accumulation():
292292
}
293293

294294
# Run 1: micro-batching enabled
295-
backend_micro.forward_backward(prepare_model_pass_batch(reqs))
295+
outputs = backend_micro.forward_backward(prepare_model_pass_batch(reqs))
296+
297+
# The SDK's custom-loss path (>= 0.31) pops "loss:sum": the per-request sum of
298+
# the elementwise losses it also receives.
299+
for request_id, output in outputs.items():
300+
expected = sum(sum(datum["elementwise_loss"]["data"]) for datum in output.loss_fn_outputs)
301+
assert output.metrics["loss:sum"] == pytest.approx(expected, rel=1e-5), request_id
296302

297303
adapter1_idx = backend_micro.models[adapter1_id].adapter_index
298304
adapter2_idx = backend_micro.models[adapter2_id].adapter_index

‎tests/tinker/test_api_validation.py‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,3 +178,73 @@ def test_nested_in_model_input(self):
178178
json_dict = types_input.model_dump(mode="json")
179179
recovered = types.ModelInput.model_validate(json_dict)
180180
assert recovered.chunks[0].data == _RAW_PNG
181+
182+
183+
# --- SDK >= 0.32 request fields SkyRL does not implement -------------------
184+
185+
186+
def _sample_request(**extra) -> api.SampleRequest:
187+
return api.SampleRequest(
188+
base_model="m",
189+
prompt=api.ModelInput(chunks=[api.EncodedTextChunk(tokens=[1, 2])]),
190+
sampling_params=api.SamplingParams(max_tokens=1),
191+
**extra,
192+
)
193+
194+
195+
def test_sample_request_accepts_default_new_options():
196+
req = _sample_request(topk_sample_logprobs=0, prompt_alt_tokens_k=0, target_prompt_logprobs=None)
197+
assert req.topk_sample_logprobs == 0
198+
199+
200+
@pytest.mark.parametrize(
201+
"extra",
202+
[
203+
{"topk_sample_logprobs": 4},
204+
{"prompt_alt_tokens_k": 2},
205+
{"target_prompt_logprobs": {"data": [1], "dtype": "int64", "shape": [1, 1]}},
206+
{"prompt_logprobs_last_n": 3},
207+
],
208+
)
209+
def test_sample_request_rejects_unsupported_options(extra):
210+
"""Rejecting beats ignoring: the SDK would otherwise return None for data the caller asked for."""
211+
with pytest.raises(ValidationError, match="Unsupported sampling options"):
212+
_sample_request(**extra)
213+
214+
215+
def test_create_model_accepts_adamw_optimizer_config():
216+
req = api.CreateModelRequest(
217+
session_id="s", base_model="m", lora_config=api.LoRAConfig(rank=1), optimizer_config={"type": "adamw"}
218+
)
219+
assert req.optimizer_config == {"type": "adamw"}
220+
221+
222+
def test_create_model_rejects_non_adam_optimizer_config():
223+
with pytest.raises(ValidationError, match="only 'adamw' is supported"):
224+
api.CreateModelRequest(
225+
session_id="s",
226+
base_model="m",
227+
lora_config=api.LoRAConfig(rank=1),
228+
optimizer_config={"type": "dimuon", "version": 1},
229+
)
230+
231+
232+
def test_load_weights_rejects_non_adam_optimizer_config():
233+
with pytest.raises(ValidationError, match="only 'adamw' is supported"):
234+
api.LoadWeightsRequest(model_id="m", path="tinker://m/weights/c", optimizer_config={"type": "dimuon"})
235+
236+
237+
def test_optim_step_legacy_adam_params_wire_key():
238+
req = api.OptimStepRequest(model_id="m", adam_params={"learning_rate": 1e-4})
239+
assert req.adam_params is not None and req.adam_params.learning_rate == 1e-4
240+
241+
242+
def test_optim_step_rejects_non_adam_optimizer_params():
243+
"""SDK >= 0.32 sends non-Adam families under ``optimizer_params``."""
244+
with pytest.raises(ValidationError, match="only Adam"):
245+
api.OptimStepRequest(model_id="m", optimizer_params={"type": "dimuon", "learning_rate": 1e-3})
246+
247+
248+
def test_optim_step_requires_adam_params():
249+
with pytest.raises(ValidationError, match="adam_params is required"):
250+
api.OptimStepRequest(model_id="m")

‎uv.lock‎

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)