From 8f77c154e243f00e4654a1ba637fc6121bf92797 Mon Sep 17 00:00:00 2001 From: Abhay Kumar Date: Sat, 24 Oct 2026 02:06:00 +0530 Subject: [PATCH] Pre-release fixes from the developer review - train checks its data files before loading any model and names the exact data split command when recipe data is missing; no output folder is left behind. - CLI config errors are one line ("unknown setting train.learning_rate (did you mean train.lr?)"); an override without = is rejected. - Exports record the length they were trained on (decision_config max_length); loaded and served models refuse longer requests unless max_length / --max-length allows them. - Every question needs a type, and the server needs a state. - README: confidence defined; install from GitHub first, PyPI once published; backbone tests use tiny models, with the real checkpoints listed; CPU latency and the Decision Index latency rule. - deciformer15k_qwen3 saves resumable state every 125 steps. - decisionformer --version. --- README.md | 32 +++++++---- src/decisionformer/cli.py | 15 ++++-- src/decisionformer/config.py | 53 +++++++++++++++++++ src/decisionformer/model.py | 2 +- .../recipes/deciformer15k_qwen3.yaml | 1 + src/decisionformer/schema.py | 4 +- src/decisionformer/serve.py | 2 +- src/decisionformer/trainer.py | 5 +- tests/test_api.py | 3 ++ tests/test_units.py | 25 ++++++++- 10 files changed, 120 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 1324e56..14767c8 100644 --- a/README.md +++ b/README.md @@ -10,17 +10,10 @@ generation. Three lines to decide, one command to train, one command to serve. Python 3.10 or newer, with PyTorch 2.6+ and transformers 5.20+. -**With pip** (from the first public release; until then, install from GitHub) - -```bash -pip install decisionformer -pip install "decisionformer[serve]" # with the API server -``` - **From GitHub** ```bash -pip install "git+https://github.com/FrontiersMindAI/decisionformer.git" +pip install "decisionformer[serve] @ git+https://github.com/FrontiersMindAI/decisionformer.git" # or, to develop git clone https://github.com/FrontiersMindAI/decisionformer.git @@ -28,6 +21,12 @@ cd decisionformer pip install -e ".[dev]" ``` +**With pip**, once v0.2.0 is published on PyPI (the name currently holds an empty placeholder): + +```bash +pip install "decisionformer[serve]" +``` + Optional extras, combined as `pip install "decisionformer[serve,qlora]"`: | extra | adds | @@ -96,7 +95,16 @@ response = model.decide( ``` `choice` picks one of the `criteria`, `noul` answers yes/no with the probability of yes, and `score` picks one of -2 to 10 ordered levels. `model.decide_many([...])` answers several requests in shared batches. +2 to 10 ordered levels; every question needs a `type`. `confidence` is not the top probability: it is 1 minus the +normalized entropy of the probabilities, from 0 (all options equally likely) to 1 (certain), so a 0.51/0.49 choice +has a confidence near 0. Checkpoints in the AutoJev format report the top probability's margin over chance instead. +`model.decide_many([...])` answers several requests in shared batches. + +A model refuses requests longer than the length it was trained on (`data.max_length`, 1,024 tokens by default) with +a `ContextLengthError` (HTTP 422 from the server) instead of truncating them; pass `max_length=` to +`from_pretrained`, or `--max-length` to `serve`, to allow longer ones. On a CPU, `vllm-sr/d3-edge` (0.6B parameters) +answers a request in several seconds; the Decision Index's latency rule (a median of at most 1,000 ms per request) +needs a GPU. ### Serve as an API @@ -113,7 +121,7 @@ curl localhost:8000/v1/systemone -H 'content-type: application/json' -d '{ }' ``` -The server takes the same `state` and `questions` and returns the same JSON as `decide`, with `id`, `object` and +The server takes the same `state` (required, may be empty) and `questions` and returns the same JSON as `decide`, with `id`, `object` and `created` added. It is the System One API used by Jev, vLLM and the Decision Index; the request and response schemas are at `/docs`. Over-long requests are refused with HTTP 422, never truncated, and `--api-key-env MY_KEY_VAR` requires `Authorization: Bearer `. A request that arrives while the model is idle @@ -130,7 +138,9 @@ decisionformer serve vllm-sr/d3-edge --backend vllm ## Supported backbones Every family below is fine-tuned, exported, reloaded and checked against the Decision Index's response rules in -`tests/test_zoo.py`, newest first within each group: +`tests/test_zoo.py`, using tiny randomly initialized models of each architecture, newest first within each group. +Real checkpoints fine-tuned, exported and reloaded so far: Qwen3-0.6B, Qwen3.5-0.8B, SmolLM2-135M and 360M, +ModernBERT-base and T5-small; `vllm-sr/d3-edge` is loaded and served (`docs/benchmarks.md`). | | families | |---|---| diff --git a/src/decisionformer/cli.py b/src/decisionformer/cli.py index 951dc90..5c22a3e 100644 --- a/src/decisionformer/cli.py +++ b/src/decisionformer/cli.py @@ -16,18 +16,22 @@ def _run_config(args: argparse.Namespace, **options: Any) -> Any: - from decisionformer.config import load_config + from decisionformer.config import explain, load_config try: return load_config(args.config, args.overrides, **options) - except FileNotFoundError as error: - raise SystemExit(f"error: {error}") from None + except (FileNotFoundError, ValueError) as error: # pydantic's ValidationError is a ValueError + raise SystemExit(f"error: {explain(error)}") from None def _train(args: argparse.Namespace) -> None: from decisionformer.trainer import train - summary = train(_run_config(args)) + config = _run_config(args) + try: + summary = train(config) + except FileNotFoundError as error: + raise SystemExit(f"error: {error}") from None if summary: print(json.dumps(summary, indent=2)) @@ -225,8 +229,11 @@ def _model_options(command: argparse.ArgumentParser) -> None: def build_parser() -> argparse.ArgumentParser: + from decisionformer import __version__ + raw = argparse.RawDescriptionHelpFormatter parser = argparse.ArgumentParser(prog="decisionformer", description=__doc__, formatter_class=raw) + parser.add_argument("--version", action="version", version=f"decisionformer {__version__}") commands = parser.add_subparsers(dest="command", required=True) examples = """examples: diff --git a/src/decisionformer/config.py b/src/decisionformer/config.py index 95ce80d..b5061a0 100644 --- a/src/decisionformer/config.py +++ b/src/decisionformer/config.py @@ -263,6 +263,9 @@ def load_config(source: str | Path | Mapping[str, Any], overrides: Sequence[str] for key, value in overrides.items(): OmegaConf.update(node, key, value, merge=True) elif overrides: + malformed = [item for item in overrides if "=" not in item] + if malformed: + raise ValueError(f"override {malformed[0]!r} needs the form key=value, e.g. train.lr=1e-4") node = OmegaConf.merge(node, OmegaConf.from_dotlist(list(overrides))) config = RunConfig.model_validate(OmegaConf.to_container(node, resolve=True)) if validate_components: @@ -270,6 +273,56 @@ def load_config(source: str | Path | Mapping[str, Any], overrides: Sequence[str] return config +def check_data_paths(config: RunConfig) -> None: + """Fail before any model is loaded when a training, evaluation or calibration file is missing.""" + if config.data.train is None: + raise ValueError("data.train is required for training") + paths: list[str] = [] + for value in (config.data.train, config.data.calibration, *config.data.eval.values()): + paths += [value] if isinstance(value, str) else list(value or []) + missing = [path for path in paths if not path.startswith("hf:") and not Path(path).expanduser().exists()] + if missing: + hint = ("Create it first: decisionformer data split hf:FrontiersMind/deciformer-15k:train " + "--out data/deciformer-15k" if "deciformer-15k" in missing[0] else + "Point data.train, data.eval and data.calibration at your JSONL files, or make them with " + "`decisionformer data split SOURCE --out DIR`") + raise FileNotFoundError(f"data file not found: {missing[0]}. {hint}") + + +# Names from other trainers (Hugging Face TrainingArguments) and the setting they mean here. +COMMON_NAMES = {"learning_rate": "lr", "batch_size": "global_batch_size", "train_batch_size": "global_batch_size", + "per_device_train_batch_size": "micro_batch_size", "num_train_epochs": "epochs", + "num_epochs": "epochs", "steps": "max_steps", "bf16": "mixed_precision", "fp16": "mixed_precision", + "model_name": "backbone", "model_name_or_path": "backbone", "max_seq_length": "max_length"} + + +def explain(error: Exception) -> str: + """A short, readable message for a config error (one line per problem, with a suggestion for typos).""" + import difflib + + from pydantic import ValidationError + + if not isinstance(error, ValidationError): + return str(error) + lines = [] + for problem in error.errors(): + location = [str(part) for part in problem["loc"]] + key = ".".join(location) + if problem["type"] == "extra_forbidden": + model: Any = RunConfig + for part in location[:-1]: + field = getattr(model, "model_fields", {}).get(part) + model = field.annotation if field is not None else None + options = list(getattr(model, "model_fields", {})) + alias = COMMON_NAMES.get(location[-1]) + close = [alias] if alias in options else difflib.get_close_matches(location[-1], options, n=1) + suggestion = f" (did you mean {'.'.join([*location[:-1], close[0]])}?)" if close else "" + lines.append(f"unknown setting {key}{suggestion}") + else: + lines.append(f"{key}: {problem['msg']}") + return "; ".join(lines) + + def validate_components_of(config: RunConfig) -> None: """Fail at load time for unknown poolers/heads, bad head arguments, or invalid LoRA settings.""" from decisionformer.finetune import lora_config_from diff --git a/src/decisionformer/model.py b/src/decisionformer/model.py index fa216b2..36af277 100644 --- a/src/decisionformer/model.py +++ b/src/decisionformer/model.py @@ -179,7 +179,7 @@ def decision_config(result: BuildResult, config: RunConfig, *, temperatures: dic "codes": list(builder.codes), "token_ids": list(builder.code_token_ids or []), "temperature": float(temperatures.get("default", 1.0)), "attention_mode": CHECKPOINT_NAMES.get(result.attention, result.attention), - "pooling": "last" if readout else "decisionformer", "max_length": None, "readout_dtype": "float32", + "pooling": "last" if readout else "decisionformer", "max_length": config.data.max_length, "readout_dtype": "float32", } if by_type: payload["temperature_by_type"] = by_type diff --git a/src/decisionformer/recipes/deciformer15k_qwen3.yaml b/src/decisionformer/recipes/deciformer15k_qwen3.yaml index f64e98f..ab561f0 100644 --- a/src/decisionformer/recipes/deciformer15k_qwen3.yaml +++ b/src/decisionformer/recipes/deciformer15k_qwen3.yaml @@ -23,3 +23,4 @@ train: head_lr: 2.0e-5 # the readout starts from the LM-head rows; keep its step small token_budget: 8192 eval_every: 125 + save_every: 125 # resumable state; rerun the same command to resume diff --git a/src/decisionformer/schema.py b/src/decisionformer/schema.py index f814651..3491765 100644 --- a/src/decisionformer/schema.py +++ b/src/decisionformer/schema.py @@ -84,7 +84,9 @@ def parse_question(key: str, raw: Any) -> Question: where = f"question {key!r}" if not isinstance(raw, dict): raise DecisionInputError(f"{where}: must be an object") - kind = _TYPE_ALIASES.get(str(raw.get("type", "choice"))) + if "type" not in raw: + raise DecisionInputError(f"{where}: needs a `type`: one of {QUESTION_TYPES}") + kind = _TYPE_ALIASES.get(str(raw["type"])) if kind is None: raise DecisionInputError(f"{where}: unknown type {raw.get('type')!r}; use one of {QUESTION_TYPES}") criteria = raw.get("criteria", raw.get("options")) diff --git a/src/decisionformer/serve.py b/src/decisionformer/serve.py index 3e0e82b..c5bea0a 100644 --- a/src/decisionformer/serve.py +++ b/src/decisionformer/serve.py @@ -152,7 +152,7 @@ async def systemone(request: Request) -> Any: body = await request.json() except ValueError: return error(400, "the body must be JSON", "invalid_request") - if not isinstance(body, dict) or "questions" not in body: + if not isinstance(body, dict) or "questions" not in body or "state" not in body: return error(400, "a request needs `state` and `questions`", "invalid_request") started = time.perf_counter() try: diff --git a/src/decisionformer/trainer.py b/src/decisionformer/trainer.py index 5ca58d8..8622ee6 100644 --- a/src/decisionformer/trainer.py +++ b/src/decisionformer/trainer.py @@ -18,7 +18,7 @@ from torch.nn.parallel import DistributedDataParallel from decisionformer import distributed as dist_utils -from decisionformer.config import RunConfig, dump_config, load_config +from decisionformer.config import RunConfig, check_data_paths, dump_config, load_config from decisionformer.data import Collator, DecisionDataset, plan_epoch from decisionformer.finetune import count_parameters, effective_precision from decisionformer.inference import ( @@ -98,12 +98,11 @@ def __init__(self, config: RunConfig) -> None: self.config = config self.state = dist_utils.setup(config.distributed.backend, config.distributed.timeout_minutes) _setup_logging(self.state.is_main) + check_data_paths(config) # before any download or output folder self.output = Path(config.train.output_dir) if self.state.is_main: self.output.mkdir(parents=True, exist_ok=True) (self.output / "config.yaml").write_text(dump_config(config)) - if config.data.train is None: - raise ValueError("data.train is required for training") _seed_everything(config.train.seed) self.result: BuildResult = build_model(config, self.state.device, training=True) self.model = self.result.model diff --git a/tests/test_api.py b/tests/test_api.py index f6de28f..fc34e89 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -61,6 +61,7 @@ def test_standard_export_loads_and_decides(trained): config = json.loads((best / "decision_config.json").read_text()) assert config["format_id"] == "d3-code-readout-v1" and config["prompt"] == "d3" assert config["attention_mode"] == "causal" and config["pooling"] == "last" and len(config["token_ids"]) == 255 + assert config["max_length"] == 256, "exports record the length they were trained on (the serving limit)" assert set(config["temperature_by_type"]) == {"choice", "noul", "score"} assert (best / "readout.safetensors").exists() and (best / "model.safetensors").exists() card = (best / "README.md").read_text() @@ -181,6 +182,8 @@ def test_systemone_server(trained): index_validate(QUESTIONS, response.json()) too_long = client.post("/v1/systemone", json={"state": " ".join(["red"] * 300), "questions": {"q": {"type": "noul"}}}) assert too_long.status_code == 422 and "maximum context length" in too_long.text + no_state = client.post("/v1/systemone", json={"questions": {"q": {"type": "noul"}}}) + assert no_state.status_code == 400 and "state" in no_state.text bad = client.post("/v1/systemone", json={"state": "x", "questions": {"q": {"type": "score", "criteria": ["a"]}}}) assert bad.status_code == 400 and "a score takes 2 to 10 levels" in bad.text schema = client.get("/openapi.json").json() diff --git a/tests/test_units.py b/tests/test_units.py index 2df2515..87e4165 100644 --- a/tests/test_units.py +++ b/tests/test_units.py @@ -61,7 +61,7 @@ def test_question_errors(raw, message): def test_answer_errors(): with pytest.raises(DecisionInputError, match="not one of the options"): - parse_request({"state": "s", "questions": {"q": {"criteria": {"a": None}}}, "gold": {"q": "z"}}) + parse_request({"state": "s", "questions": {"q": {"type": "choice", "criteria": {"a": None}}}, "gold": {"q": "z"}}) # ---------------------------------------------------------------------------------------------------- formats @@ -473,3 +473,26 @@ def test_local_runs_answer_with_their_run_name(tmp_path): best.mkdir(parents=True) assert display_name(best, best) == "my-decider" assert display_name("org/model", best) == "org/model" + + +def test_questions_need_a_type(): + with pytest.raises(DecisionInputError, match="needs a `type`"): + parse_question("q", {"criteria": ["a", "b"]}) + + +def test_config_errors_are_short_and_suggest_fixes(tmp_path, monkeypatch): + from pydantic import ValidationError + + monkeypatch.chdir(tmp_path) # the recipe's relative data paths do not exist here + + from decisionformer.config import check_data_paths, explain + + with pytest.raises(ValidationError) as caught: + load_config("deciformer15k_qwen3", ["train.learning_rate=1e-4"]) + assert explain(caught.value) == "unknown setting train.learning_rate (did you mean train.lr?)" + with pytest.raises(ValueError, match="needs the form key=value"): + load_config("deciformer15k_qwen3", ["train.lr"]) + missing = load_config("deciformer15k_qwen3", [f"train.output_dir={tmp_path / 'run'}"]) + with pytest.raises(FileNotFoundError, match="decisionformer data split hf:FrontiersMind/deciformer-15k"): + check_data_paths(missing) + assert not (tmp_path / "run").exists()