Repository navigation
Add comprehensive testing guide documentation - #5
Merged
Merged
Conversation
Document the test suite structure, how to run tests, and when to run which groups. Covers unit tests, distributed tests requiring torchrun, CUDA-gated tests, benchmarks, shared fixtures, and recommended workflows. https://claude.ai/code/session_017oY6i89L7mUfSTQ75hnra6
jdinalt
added a commit
that referenced
this pull request
Apr 19, 2026
Implemented fixes for issues identified in comprehensive code review:
1. **Removed batch buffer from length estimation** (lines 984-989)
- Prevents race condition in multi-worker scenarios where
_current_batch_buffer_size is not synchronized across processes
- Simplifies logic while maintaining accurate progressive estimates
2. **Use full SHA256 hash for fingerprints** (lines 1145, 1193)
- Changed from 16-char truncated hash to full 64-char hash
- Eliminates collision risk for datasets with many configurations
- Negligible storage cost for better safety
3. **Added shard index validation** (lines 358-363)
- Validates index < num_shards before processing
- Prevents silent empty shard creation from invalid parameters
- Clear error messages for out-of-range indices
4. **Added batch map result validation** (lines 624-645)
- Validates that batch map functions return consistent list lengths
- Catches common error of returning scalars instead of lists
- Prevents cryptic indexing errors during iteration
- Allows intentional scalar broadcasting for metadata fields
Note: Length invalidation flag inheritance (#5) was already handled
correctly - both slice() and shard() reset _length_invalidated = False.
Issues documented but not fixed (require design changes):
- Map composition with different configurations (needs refactoring)
- State dict doesn't preserve map config (design limitation)
- Column names not updated after map (design limitation)
All 51 unit tests pass.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
3 of 5 tasks
jdinalt
added a commit
that referenced
this pull request
May 20, 2026
Backend (inference server) - service.py: replace fastapi.HTTPException raised from _resolve_entry with a domain ModelNotFoundError; the service module stays transport-agnostic and the route layer translates to 404 via a new exception_handler in routes.py. (P3 #9) - service.py: --eager-load loads every entry at startup so a broken checkpoint surfaces at boot instead of waiting for first request to land on it. (P3 #12) - routes.py: every per-route catch-all now lets ModelNotFoundError bubble through to the app handler so a wrong "model" field returns 404 rather than the catch-all's 500. Backend (cluster picker) - routes/cluster.py: rewrote the /inference_servers docstring — it used to claim "token-stripped" while the schema and serializer actually shipped the token. New docstring matches reality and points at the schema's rationale + follow-up note. (P1 #1) - cluster_inference_inventory.py: token_for_url canonicalizes via urlparse before matching — lowercased scheme + host, default ports stripped, IPv6 brackets restored. URLs like "HTTP://Host:80" now find an entry stored as "http://host". (P3 #8) - cluster_inference_inventory.py: wake_loops docstring updated to reflect the now-wired scheduler hook. (P1 #3) - scheduler.py: new _wake_inference_inventory() helper called on inference-job launch and reap (regular + abort paths) so the cluster picker converges in ~1s rather than waiting for the 10s steady-state collect tick. (P1 #3) - routes/inference_proxy.py: hoist cluster_inference_inventory import to module top and drop the bare-except wrapper. A typo in the inventory module now raises at import time instead of silently returning None for every off-host token lookup. (P2 #7) WebUI - InferenceModelPanel.tsx: gate jobsQ and clusterServersQ on clusterSelfQ.isFetched so the picker doesn't briefly show local- only state and then swap when the cluster gate resolves. Renders a "Loading…" placeholder for the brief gate-resolving window rather than the misleading "no live inference jobs" empty state. (P1 #2) - InferenceModal.tsx: resetDefaults now also clears priority, matching the "back to first-open state" intent. (P3 #10) - styles.css: .inference-model-rows-scroll uses max(240px, 30vh) so tall screens get more breathing room. (P3 #11) Tests - New tests/unit/forgather_server/test_routes_cluster_inference.py (9 tests): GET /inference_servers_local + tokens + 401 path; GET /inference_servers master path + non-master proxy fallback + pinned token-exposure assertion; POST /inference_servers/refresh wake-event firing + 401; _PEER_ALLOWED_PATHS membership for all three new paths. (P1 #4, P3 #13) - New tests/unit/forgather_server/test_cluster_inference_inventory_ master.py (12 tests): _collect_servers_tick local + peer-fanout + peer-error tolerance; _health_tick 200 / non-200 / connect-error / consecutive-failure-counter reset / zero-server pass-complete (also verifies the /health URL, not /v1/health); _sync_master_ state across all three cluster states; wake_loops fans out to every registered event. (P2 #5) - Extended tests/unit/forgather_server/test_inference_proxy.py with four cases for the new off-host branch: cluster-inventory hit, empty-inventory-returns-None, URL canonicalization, and a sanity test that the local loopback fast path is unaffected. (P2 #6) - Updated tools/inference_server/tests/test_multi_model.py to assert ModelNotFoundError (with .requested / .available) instead of HTTPException after the decoupling. All 164 backend tests green (66 cluster/inference + 33 proxy + 30 multi-model + 35 dataset-side); webui typecheck clean. Live-smoke: --eager-load loads both models at startup; unknown "model" field returns HTTP 404 with the new ModelNotFoundError payload. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jdinalt
added a commit
that referenced
this pull request
May 20, 2026
* inference: multi-model server with CPU↔GPU swap One inference-server process can now host multiple models. Requests dispatch by the OpenAI 'model' field; models lazy-load on first use and swap between CPU and GPU memory under an asyncio lock. Single- model setups keep their existing eager-load behavior. Backend (tools/inference_server/) - service.py: new ModelEntry dataclass holds per-model state (tokenizer, weights, generation config, chat template, stop tokens, per-model utility objects). InferenceService becomes a registry with @Property shims so existing strategies still read service.model / service.tokenizer / etc. unchanged. New async acquire(name) context manager performs lazy-load + swap. - routes.py: /v1/models lists all configured entries with an x_state field (unloaded / cpu / gpu). All generation routes (chat, completion, scoring fast-path, tokenize) wrap the strategy in acquire(); streaming holds the lock for the full SSE stream. - server.py: -m / --model becomes repeatable, accepting PATH or NAME=PATH. Rejects -c <path> and device=auto with multi-model; warns on --compile with multi-model. New --keep-on-gpu flag disables CPU demotion so all models stay GPU-resident. - config.py: YAML 'models:' list with per-model overrides (dtype, stop_sequences, chat_template, etc.). CLI -m takes precedence. - core/generation_logger.py: tokenizer becomes a property reading through a factory so log decodes follow the active model. Wire path - src/forgather/cli/inference.py: forgather inf server --enqueue accepts -m repeatedly and --keep-on-gpu; packs 'models' list into job_params when more than one model is specified. - tools/forgather_server/{inference_ops,launcher,scheduler}.py: thread 'models' and 'keep_on_gpu' through to argv. WebUI (InferenceModal.tsx) - Multi-model UX: "+ Add model" button in ad-hoc mode adds rows; each row has a path picker and optional name override. Hidden in project-backed flows (a project is a single model). The specific-checkpoint-path field is hidden when more than one model is configured. - Scrollable row container so the modal doesn't outgrow the viewport when many models are added. - "Keep all models on GPU" checkbox (visible in multi-model mode). - "Create service…" suggested name: model basename for single model, host-port for multi-model (concatenating names doesn't scale). - PathField gains a rememberKey prop that persists the parent of the last-picked path under a localStorage key; the next browse with an empty value lands at that parent — natural for "add another sibling" workflows like multi-model selection. Tests - tools/inference_server/tests/test_multi_model.py covers CLI arg parsing, YAML entry parsing, CLI/YAML reconciliation, service construction validation, name resolution, the acquire+swap protocol, and --keep-on-gpu. 30 tests, all green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * cluster: aggregate inference servers across peers The webui's "Running inference servers" picker only surfaced jobs spawned on the local peer. In cluster mode, jobs running on other peers were invisible — operators had to type the URL and bearer token manually. This change mirrors the dataset-server inventory pattern so the picker shows the full cluster. Backend - tools/forgather_server/cluster_inference_inventory.py (new): ``LocalInference`` + ``MasterInventory`` modeled on ``cluster_dataset_inventory``. Per-peer JobRecord scan with the multi-model-aware ``models[]`` extraction (single ``-m`` path basename or per-entry names from ``-m NAME=PATH``), 0.0.0.0 → cluster hostname rewriting, loopback flagging. Two async loops (collect from peers, /health probe) self-gate on master role. - routes/cluster.py: GET /api/cluster/inference_servers_local (per-peer, tokens, peer-mTLS allowed), GET /api/cluster/ inference_servers (master-aggregated, non-master proxies to master), POST /api/cluster/inference_servers/refresh (wake hook). - routes/inference_proxy.py: ``_token_for`` consults the cluster inventory's ``token_for_url`` for off-host URLs. On master nodes this auto-attaches the upstream bearer; on non-master nodes the inventory is empty and the webui carries the token via the ``X-Inference-Auth-Token`` header. - auth.py: adds the three new cluster paths to the mTLS peer allowlists. - app.py: starts ``master_collect_servers_loop`` and ``master_health_loop`` on cluster-active boot; wires the membership listener so a master transition wakes both loops. Tradeoff: ``/api/cluster/inference_servers`` includes the bearer token in each entry. Stripping it would require either server-side token attach via a non-master pull loop or per-request master proxying — both substantially more work. The local /api/jobs endpoint already exposes per-job tokens to the browser, so this extension is consistent with today's surface, not a regression. Comments at the new types flag the follow-up. WebUI - src/api.ts: new ``ClusterInferenceServer`` type + ``getClusterInferenceServers`` method. - components/InferenceModelPanel.tsx: queries the cluster endpoint when in cluster mode and skips the local-jobs query; falls back to the local-jobs query otherwise. Picker rows unify both shapes through a new ``PickerRow`` interface. Header shows a "cluster" badge in cluster mode; rows get peer-id and health-failure indicators. Tests - tests/unit/forgather_server/test_cluster_inference_inventory.py: 15 tests covering JobRecord scan (single + multi-model param shapes, loopback, 0.0.0.0 rewrite, scheme detection, non-running skip), ``models[]`` extraction across all four input shapes, and ``MasterInventory`` health-merge + ``token_for_url`` + role transitions. All green. Adjacent dataset-inventory tests still pass (50 total). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review fixes: P1–P3 from inference multi-model + cluster picker review Backend (inference server) - service.py: replace fastapi.HTTPException raised from _resolve_entry with a domain ModelNotFoundError; the service module stays transport-agnostic and the route layer translates to 404 via a new exception_handler in routes.py. (P3 #9) - service.py: --eager-load loads every entry at startup so a broken checkpoint surfaces at boot instead of waiting for first request to land on it. (P3 #12) - routes.py: every per-route catch-all now lets ModelNotFoundError bubble through to the app handler so a wrong "model" field returns 404 rather than the catch-all's 500. Backend (cluster picker) - routes/cluster.py: rewrote the /inference_servers docstring — it used to claim "token-stripped" while the schema and serializer actually shipped the token. New docstring matches reality and points at the schema's rationale + follow-up note. (P1 #1) - cluster_inference_inventory.py: token_for_url canonicalizes via urlparse before matching — lowercased scheme + host, default ports stripped, IPv6 brackets restored. URLs like "HTTP://Host:80" now find an entry stored as "http://host". (P3 #8) - cluster_inference_inventory.py: wake_loops docstring updated to reflect the now-wired scheduler hook. (P1 #3) - scheduler.py: new _wake_inference_inventory() helper called on inference-job launch and reap (regular + abort paths) so the cluster picker converges in ~1s rather than waiting for the 10s steady-state collect tick. (P1 #3) - routes/inference_proxy.py: hoist cluster_inference_inventory import to module top and drop the bare-except wrapper. A typo in the inventory module now raises at import time instead of silently returning None for every off-host token lookup. (P2 #7) WebUI - InferenceModelPanel.tsx: gate jobsQ and clusterServersQ on clusterSelfQ.isFetched so the picker doesn't briefly show local- only state and then swap when the cluster gate resolves. Renders a "Loading…" placeholder for the brief gate-resolving window rather than the misleading "no live inference jobs" empty state. (P1 #2) - InferenceModal.tsx: resetDefaults now also clears priority, matching the "back to first-open state" intent. (P3 #10) - styles.css: .inference-model-rows-scroll uses max(240px, 30vh) so tall screens get more breathing room. (P3 #11) Tests - New tests/unit/forgather_server/test_routes_cluster_inference.py (9 tests): GET /inference_servers_local + tokens + 401 path; GET /inference_servers master path + non-master proxy fallback + pinned token-exposure assertion; POST /inference_servers/refresh wake-event firing + 401; _PEER_ALLOWED_PATHS membership for all three new paths. (P1 #4, P3 #13) - New tests/unit/forgather_server/test_cluster_inference_inventory_ master.py (12 tests): _collect_servers_tick local + peer-fanout + peer-error tolerance; _health_tick 200 / non-200 / connect-error / consecutive-failure-counter reset / zero-server pass-complete (also verifies the /health URL, not /v1/health); _sync_master_ state across all three cluster states; wake_loops fans out to every registered event. (P2 #5) - Extended tests/unit/forgather_server/test_inference_proxy.py with four cases for the new off-host branch: cluster-inventory hit, empty-inventory-returns-None, URL canonicalization, and a sanity test that the local loopback fast path is unaffected. (P2 #6) - Updated tools/inference_server/tests/test_multi_model.py to assert ModelNotFoundError (with .requested / .available) instead of HTTPException after the decoupling. All 164 backend tests green (66 cluster/inference + 33 proxy + 30 multi-model + 35 dataset-side); webui typecheck clean. Live-smoke: --eager-load loads both models at startup; unknown "model" field returns HTTP 404 with the new ModelNotFoundError payload. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jdinalt
added a commit
that referenced
this pull request
May 23, 2026
…xes (#69) * Analyze tab: per-token scoring with histogram, plus dataset shortcuts (#55) Adds an Analyze sub-tab to the webui Inference panel that visualizes per-token causal-LM loss / entropy with hover top-K alternatives, colormap choice (viridis / plasma / magma / inferno / turbo / green→red), auto or manual scale, EMA smoothing, SVG histogram, and drag-resizable splits. Inference server: implements the previously-unimplemented logprobs field on POST /v1/completions using the OpenAI legacy "score the prompt" shape (echo=true, logprobs=K, max_tokens=0). Same request shape vLLM uses, so the same webui client works against vLLM once its Transformers v5 support lands. Adds a non-standard token_entropies field (Shannon entropy in nats of the full vocab distribution) that OpenAI/vLLM don't expose; clients fall back to loss when absent. Dataset preview also got several quality-of-life improvements that pair with the analyze tab: drag-select inside a row no longer collapses it, right-click cell menu (Copy + Analyze in Inference… cross-section shortcut), and a 🎲 random-page-and-row pager button for sampling out of a large corpus. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Multi-model inference + cluster-aware picker (#57) * inference: multi-model server with CPU↔GPU swap One inference-server process can now host multiple models. Requests dispatch by the OpenAI 'model' field; models lazy-load on first use and swap between CPU and GPU memory under an asyncio lock. Single- model setups keep their existing eager-load behavior. Backend (tools/inference_server/) - service.py: new ModelEntry dataclass holds per-model state (tokenizer, weights, generation config, chat template, stop tokens, per-model utility objects). InferenceService becomes a registry with @property shims so existing strategies still read service.model / service.tokenizer / etc. unchanged. New async acquire(name) context manager performs lazy-load + swap. - routes.py: /v1/models lists all configured entries with an x_state field (unloaded / cpu / gpu). All generation routes (chat, completion, scoring fast-path, tokenize) wrap the strategy in acquire(); streaming holds the lock for the full SSE stream. - server.py: -m / --model becomes repeatable, accepting PATH or NAME=PATH. Rejects -c <path> and device=auto with multi-model; warns on --compile with multi-model. New --keep-on-gpu flag disables CPU demotion so all models stay GPU-resident. - config.py: YAML 'models:' list with per-model overrides (dtype, stop_sequences, chat_template, etc.). CLI -m takes precedence. - core/generation_logger.py: tokenizer becomes a property reading through a factory so log decodes follow the active model. Wire path - src/forgather/cli/inference.py: forgather inf server --enqueue accepts -m repeatedly and --keep-on-gpu; packs 'models' list into job_params when more than one model is specified. - tools/forgather_server/{inference_ops,launcher,scheduler}.py: thread 'models' and 'keep_on_gpu' through to argv. WebUI (InferenceModal.tsx) - Multi-model UX: "+ Add model" button in ad-hoc mode adds rows; each row has a path picker and optional name override. Hidden in project-backed flows (a project is a single model). The specific-checkpoint-path field is hidden when more than one model is configured. - Scrollable row container so the modal doesn't outgrow the viewport when many models are added. - "Keep all models on GPU" checkbox (visible in multi-model mode). - "Create service…" suggested name: model basename for single model, host-port for multi-model (concatenating names doesn't scale). - PathField gains a rememberKey prop that persists the parent of the last-picked path under a localStorage key; the next browse with an empty value lands at that parent — natural for "add another sibling" workflows like multi-model selection. Tests - tools/inference_server/tests/test_multi_model.py covers CLI arg parsing, YAML entry parsing, CLI/YAML reconciliation, service construction validation, name resolution, the acquire+swap protocol, and --keep-on-gpu. 30 tests, all green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * cluster: aggregate inference servers across peers The webui's "Running inference servers" picker only surfaced jobs spawned on the local peer. In cluster mode, jobs running on other peers were invisible — operators had to type the URL and bearer token manually. This change mirrors the dataset-server inventory pattern so the picker shows the full cluster. Backend - tools/forgather_server/cluster_inference_inventory.py (new): ``LocalInference`` + ``MasterInventory`` modeled on ``cluster_dataset_inventory``. Per-peer JobRecord scan with the multi-model-aware ``models[]`` extraction (single ``-m`` path basename or per-entry names from ``-m NAME=PATH``), 0.0.0.0 → cluster hostname rewriting, loopback flagging. Two async loops (collect from peers, /health probe) self-gate on master role. - routes/cluster.py: GET /api/cluster/inference_servers_local (per-peer, tokens, peer-mTLS allowed), GET /api/cluster/ inference_servers (master-aggregated, non-master proxies to master), POST /api/cluster/inference_servers/refresh (wake hook). - routes/inference_proxy.py: ``_token_for`` consults the cluster inventory's ``token_for_url`` for off-host URLs. On master nodes this auto-attaches the upstream bearer; on non-master nodes the inventory is empty and the webui carries the token via the ``X-Inference-Auth-Token`` header. - auth.py: adds the three new cluster paths to the mTLS peer allowlists. - app.py: starts ``master_collect_servers_loop`` and ``master_health_loop`` on cluster-active boot; wires the membership listener so a master transition wakes both loops. Tradeoff: ``/api/cluster/inference_servers`` includes the bearer token in each entry. Stripping it would require either server-side token attach via a non-master pull loop or per-request master proxying — both substantially more work. The local /api/jobs endpoint already exposes per-job tokens to the browser, so this extension is consistent with today's surface, not a regression. Comments at the new types flag the follow-up. WebUI - src/api.ts: new ``ClusterInferenceServer`` type + ``getClusterInferenceServers`` method. - components/InferenceModelPanel.tsx: queries the cluster endpoint when in cluster mode and skips the local-jobs query; falls back to the local-jobs query otherwise. Picker rows unify both shapes through a new ``PickerRow`` interface. Header shows a "cluster" badge in cluster mode; rows get peer-id and health-failure indicators. Tests - tests/unit/forgather_server/test_cluster_inference_inventory.py: 15 tests covering JobRecord scan (single + multi-model param shapes, loopback, 0.0.0.0 rewrite, scheme detection, non-running skip), ``models[]`` extraction across all four input shapes, and ``MasterInventory`` health-merge + ``token_for_url`` + role transitions. All green. Adjacent dataset-inventory tests still pass (50 total). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review fixes: P1–P3 from inference multi-model + cluster picker review Backend (inference server) - service.py: replace fastapi.HTTPException raised from _resolve_entry with a domain ModelNotFoundError; the service module stays transport-agnostic and the route layer translates to 404 via a new exception_handler in routes.py. (P3 #9) - service.py: --eager-load loads every entry at startup so a broken checkpoint surfaces at boot instead of waiting for first request to land on it. (P3 #12) - routes.py: every per-route catch-all now lets ModelNotFoundError bubble through to the app handler so a wrong "model" field returns 404 rather than the catch-all's 500. Backend (cluster picker) - routes/cluster.py: rewrote the /inference_servers docstring — it used to claim "token-stripped" while the schema and serializer actually shipped the token. New docstring matches reality and points at the schema's rationale + follow-up note. (P1 #1) - cluster_inference_inventory.py: token_for_url canonicalizes via urlparse before matching — lowercased scheme + host, default ports stripped, IPv6 brackets restored. URLs like "HTTP://Host:80" now find an entry stored as "http://host". (P3 #8) - cluster_inference_inventory.py: wake_loops docstring updated to reflect the now-wired scheduler hook. (P1 #3) - scheduler.py: new _wake_inference_inventory() helper called on inference-job launch and reap (regular + abort paths) so the cluster picker converges in ~1s rather than waiting for the 10s steady-state collect tick. (P1 #3) - routes/inference_proxy.py: hoist cluster_inference_inventory import to module top and drop the bare-except wrapper. A typo in the inventory module now raises at import time instead of silently returning None for every off-host token lookup. (P2 #7) WebUI - InferenceModelPanel.tsx: gate jobsQ and clusterServersQ on clusterSelfQ.isFetched so the picker doesn't briefly show local- only state and then swap when the cluster gate resolves. Renders a "Loading…" placeholder for the brief gate-resolving window rather than the misleading "no live inference jobs" empty state. (P1 #2) - InferenceModal.tsx: resetDefaults now also clears priority, matching the "back to first-open state" intent. (P3 #10) - styles.css: .inference-model-rows-scroll uses max(240px, 30vh) so tall screens get more breathing room. (P3 #11) Tests - New tests/unit/forgather_server/test_routes_cluster_inference.py (9 tests): GET /inference_servers_local + tokens + 401 path; GET /inference_servers master path + non-master proxy fallback + pinned token-exposure assertion; POST /inference_servers/refresh wake-event firing + 401; _PEER_ALLOWED_PATHS membership for all three new paths. (P1 #4, P3 #13) - New tests/unit/forgather_server/test_cluster_inference_inventory_ master.py (12 tests): _collect_servers_tick local + peer-fanout + peer-error tolerance; _health_tick 200 / non-200 / connect-error / consecutive-failure-counter reset / zero-server pass-complete (also verifies the /health URL, not /v1/health); _sync_master_ state across all three cluster states; wake_loops fans out to every registered event. (P2 #5) - Extended tests/unit/forgather_server/test_inference_proxy.py with four cases for the new off-host branch: cluster-inventory hit, empty-inventory-returns-None, URL canonicalization, and a sanity test that the local loopback fast path is unaffected. (P2 #6) - Updated tools/inference_server/tests/test_multi_model.py to assert ModelNotFoundError (with .requested / .available) instead of HTTPException after the decoupling. All 164 backend tests green (66 cluster/inference + 33 proxy + 30 multi-model + 35 dataset-side); webui typecheck clean. Live-smoke: --eager-load loads both models at startup; unknown "model" field returns HTTP 404 with the new ModelNotFoundError payload. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: multi-model inference + cluster picker (#58) Cover the new surface across both technical and user-facing docs, and add a CLAUDE.md reminder that docs ship in the same PR as the feature. CLAUDE.md - New "Documentation updates ship with the feature" note: both layers (technical docs in tools/<component>/, user-facing in docs/) update alongside the code, in the same PR by default. Inference server technical docs (tools/inference_server/) - README.md: -m repeatable PATH or NAME=PATH in Quick Start + Options; multi-model invariants; YAML "models:" list with per-entry overrides and global ``keep-on-gpu`` / ``eager-load``; /v1/models surfaces the ``x_state`` + ``x_model_path`` Forgather extensions; Limitations rewritten to document the per-registry lock and "no batching" reality; Features bullet added for multi-model hosting. - ARCHITECTURE.md: new "Multi-Model Registry Pattern" section documenting ModelEntry / acquire() / lifecycle states / keep_on_gpu / eager_load. Service Layer rewritten to describe the registry + property shims + ModelNotFoundError translation. Request Flow diagrams updated to show acquire() wrapping the strategy in both non-streaming and streaming paths. InferenceService Core Components rewritten for the new constructor + methods. Known Limitations + Future Improvements updated for the post-multi-model state. Forgather server technical docs (tools/forgather_server/README.md) - Inference enqueue examples now show multi-model + --keep-on-gpu. - Inference panel description notes the cluster-aware picker, the flicker-free cluster gate, peer-id badge, and unhealthy warning. - InferenceModal multi-model UX documented: "+ Add model" button, scrollable row container, per-field rememberKey, host-port suggested service name, hidden specific-checkpoint field in multi-model mode. - New "Cluster inference picker" subsection: cluster_inference_ inventory.py structure, LocalInference / MasterInventory, the three endpoints + peer-allowlist carve-outs, scheduler wake hook, and the deliberate token-exposure tradeoff with the follow-up note. - Background-loop table extended with the two new inference loops (collect + health) plus the dataset side now lists module-qualified names for clarity. User-facing docs (docs/) - README.md: index links to the inference server README + ARCHITECTURE under "Models and inference." - guides/server-cli.md: multi-model enqueue examples, including --keep-on-gpu. - guides/forgather-server-walkthrough.md: cluster-mode picker callout on the model-selection step + a tip on the sidebar Inference… modal for multi-model setups. docs/tools/inference_server/{README,ARCHITECTURE}.md and docs/forgather-server.md are symlinks to the canonical files in tools/, so the mkdocs site picks up these updates automatically. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Single-spec passthrough for forgather eval test (#60) * refactor: single-spec passthrough for forgather eval test Previously a single eval CLI flag had to be declared in five places: 1. src/forgather/cli/eval_args.py (the `test` subparser) 2. src/forgather/cli/eval.py (cmd.extend forwarding) 3. src/forgather/cli/eval.py (job_params for --enqueue) 4. tools/forgather_server/eval_ops.py (build_eval_command kwargs) 5. tools/forgather_server/launcher.py + tools/forgather_server/scheduler.py (spawn_eval_process plumbing) 6. scripts/eval_script.py (the script's own argparse) Adding --fused-loss surfaced this — the user had to touch four of those sites and still ended up with a subtle bug (typo'd parser.add_argument on the wrong parser, so --fused-loss never appeared in --help). Collapse all of it to a single spec table `_EVAL_SCRIPT_ARGS` in eval_args.py. Three small helpers consume the spec: add_eval_script_args(parser) — register on any parser forward_eval_script_args(args) — emit CLI tokens from Namespace forward_eval_script_args_from_params(p) — emit CLI tokens from job_params eval_script_args_to_job_params(args) — build the enqueue payload The server-side trio (build_eval_command / spawn_eval_process / _build_eval) collapses to a pass-through chain that funnels the job_params dict into forward_eval_script_args_from_params, so the spec is the only thing that has to change to add a new flag. scripts/ eval_script.py and the CLI's `test` subparser both call add_eval_script_args() to declare the same args once. To add a new passthrough flag now: append one entry to _EVAL_SCRIPT_ARGS and use args.<dest> in eval_script.py. That's it. Verified: - `forgather eval test --help` shows --fused-loss - Local dry-run forwards --fused-loss correctly - End-to-end `forgather eval test --fused-loss ...` runs to completion - Server-side cross-check: local cmd-tail and build_eval_command argv are byte-identical for the same logical input - All 13 tests in tests/unit/forgather_server/test_eval_ops.py pass without modification (existing kwargs interface preserved via **passthrough) - Added tests for --fused-loss and for unknown-kwarg drop semantics Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: address spec-driven forwarding feedback Address review feedback on PR #60: - Local-CLI and enqueue paths disagreed on whether ``--batch-size 0`` was a legitimate value: the local path used ``is not None`` while the enqueue path used a truthy check, silently dropping 0. Introduce a third ``enqueue_when`` mode, ``"not_none"``, and switch the numeric value args (``--batch-size``, ``--max-length``, ``--stride``) to use it. - Harden ``"always"`` forward mode against an explicit ``None`` in a future job_params payload (would have emitted the literal string ``"None"``). Fall back to the spec default in that case. - Add ``_validate_spec()`` at import time. Catches enum-like typos (``forward``, ``enqueue_when``) and duplicate ``enqueue_key`` values at module load, instead of silently no-op'ing the first time the affected flag is used. This is the bug class the spec exists to prevent. - Add ``passthrough_enqueue_keys()`` accessor and use it in ``scheduler._build_eval`` to whitelist forwarded keys. Defense-in-depth against future webui/queue fields (routing, accounting, etc.) leaking into the inner-script kwargs via ``**p``. The forwarder itself already ignores unknown keys; this keeps the kwarg surface honest too. - Add tests: - Namespace-path ``--checkpoint`` rename: verifies the dest/enqueue_key divergence doesn't break the local CLI path. - ``--batch-size 0`` round-trips through both paths. - Cross-helper byte-equality: ``forward_eval_script_args(args) == forward_eval_script_args_from_params(eval_script_args_to_job_params(args))`` for a representative arg set. Locks the contract against future renames desyncing the two helpers. - ``_validate_spec()`` raises on bad ``forward`` mode and duplicate ``enqueue_key``. All 581 tests under ``tests/unit/cli`` + ``tests/unit/forgather_server`` pass. End-to-end dry-run output unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * webui: surface --fused-loss in EvalModal The shared spec already exposes fused_loss through the enqueue payload (``forgather.cli.eval_args._EVAL_SCRIPT_ARGS``); add a matching "fused loss" checkbox to the EvalModal so webui-submitted eval jobs can opt into the fused linear-cross-entropy path. Persists across ad-hoc invocations alongside the other run-parameter toggles. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Handle HF-deduplicated tied weights in load_checkpoint (#59) * fix: handle HF-deduplicated tied weights in load_checkpoint HF safetensors cannot represent shared storage, so tied weight groups (e.g. Gemma3's lm_head.weight tied to model.embed_tokens.weight) are deduplicated on save — only the canonical name lands on disk. Strict loads were rejecting these checkpoints for the missing aliases, breaking forgather eval on tied-embedding models. Detect tied groups in the in-memory module via create_sharing_metadata and treat the missing aliases as already-covered: the in-place copy_ of the canonical tensor updates the shared storage, and the caller's tie_weights() step restores sharing if assign-mode rebinds parameters. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: document assign=True contract, expand test coverage Address review feedback on PR #59: - _synthesize_tied_aliases docstring now spells out the assign=True contract (storage shared, identity broken, caller must tie_weights() before any code path that depends on Parameter identity), and names the non-trainer callers (finalize.py, update_model/update.py) that carry the same responsibility. - Add optional sharing_metadata parameter so callers that already have it don't re-walk the module. - New tests: sharded-path negative case (untied missing key still fails strict), and assign=True load against a tied + deduped checkpoint (verifies storage sharing without identity, and that manual retie restores identity). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: note pipeline-parallel caveat in _tied_aliases_in_module Per-stage calls to ``create_sharing_metadata`` cannot see cross-stage tied groups because ``id_to_fqn`` only walks the passed module. Not reachable from current workflows but worth documenting for the next maintainer who hits an HF-deduped checkpoint loaded into a pipeline- split model. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Add BPB / BPC to forgather eval test for cross-tokenizer comparison Per-token perplexity isn't comparable across models with different tokenizers: a coarser BPE drops perplexity mechanically because the denominator (tokens) changes, not because the model is better. This adds bits-per-byte (BPB) and bits-per-character (BPC) to every eval run alongside the existing perplexity, plus the tokens_per_byte conversion factor that makes the source of any past confusion explicit. BPB is the de facto standard tokenizer-agnostic metric (Pile, GPT-3, Karpathy's autoresearch). Byte/char counts come from a rank-0 pre-pass that rebuilds the eval dataset (to avoid advancing an iterable dataset's cursor) and decodes input_ids via tokenizer.decode(skip_special_tokens=True). The example cap respects the trainer's effective dispatch_eval_batches mode (default DDP dispatch caps at max_steps * batch_size; all-shards DDP caps per-rank, so * world_size). Downstream mirrors updated in lock-step: - tools/forgather_server/routes/models.py EvalResultModel - tools/forgather_server/webui/src/api.ts EvalResultData - tools/forgather_server/webui/src/components/EvalResultTable.tsx - tools/forgather_server/webui/src/components/EvalDetailPanel.tsx (bpb chip next to ppl) Known approximation: bpb = eval_loss × tokens_per_byte / ln(2). The trainer reports a mean of per-step-mean losses; this equals the true token-mean exactly when batches have uniform valid-token counts (the common case for eval with fixed max_length and full-length sequences) and is otherwise <1% off. Making it exact would require accumulating token-weighted loss inside the trainer's eval loops; deferred. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Make defining (packed) datasets easier (#62) * Make defining (packed) datasets easier [Problems] - Creating definitions for basic datasets, especially packed ones, is more difficult that need-be. - Our standard training template does not support easily using different datasets for train and validation. [Solutions] A new packed_load_dataset.yaml template has been created, which is easily interchangeable with the base template, load_dataset.yaml. Just change the name of the template to extend and you get a packed dataset. - While doing so, I moved the common dynamic args into the base template, as these are now part of the protocol. This significantly reduces the number of copies of those definitions. - Simplified the library by making the defaults configs for most datasets of the "packed" type and deleted the now redundant packed versions. Less code to maintain! And datasets using the fast-packed format have proven to be reliable and a substantial performance win. - Some of the legacy datasets, with references from examples to both the packed and non-packed versions still have both. Addressing this is a larger refactor and I would like to limit the scope of this pass. - Having eliminated the separate packed version of C4, the reference to the packed version in the evaluation projects was updated to point to the now packed base version. On the matter of split train / validation datasets, our training template can now specify a separate "eval" dataset, but this defaults to the train dataset. A few other minor changes to meta-data was also made. * Clean up loose ends from the packed-dataset refactor - Update READMEs and a template comment to drop references to the packed/non-packed config pairs that were collapsed in e0e254cf (allenai, open-thoughts, ajibawa-2023, evaluation/common, packed_chat_template.yaml). - Switch openthoughts.yaml to extend datasets/load_packed_dataset.yaml so it picks up the standard packed batch_size (250 vs the 32 it had been inheriting from the non-packed base, well below the 1000 the deleted openthoughts-packed.yaml used). Also restore stride: {{ stride | default(0) }} -- the previous commit accidentally hardcoded it to 0, silently breaking --stride. - Drop the now-redundant ns.load_method overrides from the five ajibawa-2023 configs (load_packed_dataset.yaml -- and now even base load_dataset.yaml -- already set fast_load_iterable_dataset). - Add trailing newlines to eight files touched by the previous commit (some pre-existing, some introduced). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Remove broken docs symlinks for deleted packed dataset configs The mkdocs CI job failed on the first broken symlink it encountered (docs/examples/datasets/Open-Orca/templatelib/packed.yaml), because mkdocs walks docs/ and tries to copy every file it finds as a static asset -- including symlinks whose targets no longer exist. Delete the eight symlinks under docs/ that point at *-packed.yaml / packed.yaml dataset configs deleted in e0e254cf. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Add float8 small-llm config stub (#63) Minimal config for running the small-llm pretrain project with a float8 model variant, so the path is available for testing on hardware that supports it. Plan is to flesh this out with the actual float8 settings and docs before merging back to main. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * CPU-only support: training, eval, inference end-to-end without a CUDA driver (#64) * docker/run: clean up half-created container on docker-run failure When `docker/run` is invoked on a host with no NVIDIA driver, the default `--gpus all` fails with "failed to discover GPU vendor from CDI" but leaves the container in a half-created state. The next invocation (e.g. `GPUS=none docker/run`) would then take the "existing container -> docker start" path and re-apply the bad create-time GPU config, silently ignoring the new env var. Operators had to know to run `docker/run --rm` first. The wrapper now removes the partial container on failure and prints a hint pointing at `GPUS=none` so `GPUS=none docker/run` Just Works on the retry. Also documents `docker/build --cpu` and the no-NVIDIA-driver path in the install guide, which previously only mentioned `GPUS=none` as a "common override" without flagging it as the required mode for GPU-less hosts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * preprocess: cache get_peak_hardware_flops, silence warning on CPU-only hosts Two follow-on issues with the fallback warning: 1. The function is called ~5 times per `forgather train` invocation (dynamic-arg schema scan, required-args check, bounds validation, final materialization, ...), so on a host without a recognized GPU the warning fires 5+ times back-to-back. Cache the resolved value at module scope so subsequent calls return immediately. 2. On a CPU-only host (no CUDA visible at all) the warning is just noise: MFU is meaningless on CPU regardless of the placeholder. Suppress the warning when `_detect_gpu_flops` returns a None device_name. An *unrecognized* GPU (device_name set, flops=None) still warns -- the operator can fix that by adding a hardware.yaml entry. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * train CLI: add --nproc override, auto-fall back from 'gpu' to 1 on CPU-only The lm_training_project template defaults nproc_per_node to "gpu", which torchrun resolves to the visible CUDA device count. On a host with no CUDA device (CPU-only torch build, --gpus none under docker, etc.) torchrun aborts immediately with: ValueError: invalid literal for int() with base 10: 'gpu' Two changes in the `forgather ... train` CLI: * `-n / --nproc N` lets the operator override --nproc-per-node from the command line (accepts the same values torchrun does -- integer, "gpu", or "auto"). Honored on both the local and --enqueue paths. * When --nproc isn't passed and the config asks for "gpu" but no CUDA device is visible, fall back to --nproc-per-node 1 with a warning instead of letting torchrun crash. Useful for debugging training on a laptop / Chromebook without changing project configs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * distributed: force gloo backend on CPU fallback, regardless of caller's spec DistributedEnvironment honored whatever backend the caller passed in, and the default template ships backend="cuda:nccl,cpu:gloo" (a compound spec meant to pick the right backend per device). On a CPU-only torch build the NCCL backend isn't compiled in at all, and init_process_group aborts with "Distributed package doesn't have NCCL built in" the moment NCCL is mentioned -- the device-aware dispatch doesn't help because the failure happens during backend registration, before any device is selected. When the device falls back to CPU (no accelerator visible), unconditionally override the backend to gloo. Gloo is the correct backend for CPU ranks and is always available. Log at INFO if we're overriding a non-gloo value the caller explicitly asked for so the substitution isn't silent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * modelsrc/transformer: drop cache_position from create_causal_mask call transformers 5.9 removed `cache_position` from the create_causal_mask / create_sliding_window_causal_mask signature ("Deprecated and unused.") and shifted the 4th positional slot to `past_key_values`. The existing call passed `cache_position` positionally and `past_key_values=` as a kwarg, which on 5.9 collides: TypeError: create_causal_mask() got multiple values for argument 'past_key_values' Drop the cache_position forward (the helper derives it from past_key_values internally), and pass past_key_values by keyword so the call works on both the pre-5.9 signature (where past_key_values was kwarg-only) and the current one (where it's the 4th positional). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * inference server: default --device to 'auto' instead of 'cuda:0' Previously the inference server hard-coded a CUDA device default, so `forgather inf server -m MODEL` aborted on a host with no CUDA device (CPU-only torch build, --gpus none under docker) unless the operator passed an explicit `-d cpu`. The downstream loader already handles `auto` correctly -- it forwards to HF's device_map='auto', which picks CPU when no accelerator is visible. Switch the default to `auto`. dtype resolution already returns float32 on CPU when --dtype is unspecified, so the server now starts and serves OpenAI-compatible requests on a CPU-only host out of the box. (Operators on GPU hosts get the same behaviour as before -- HF's auto picks cuda:0 when CUDA is available.) Verified end-to-end on the CPU dev container with sshleifer/tiny-gpt2: model loads on device cpu / torch.float32, /health responds, /v1/chat/completions returns a real completion. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * inference server: detect a real default device, not 'auto' or hard-coded cuda:0 Walking back the previous commit's default of "auto". Two problems with it: 1. "auto" is not a real torch.device string. HF's from_pretrained accepts it (via accelerate's device_map='auto'), but the native Forgather checkpoint loader (forgather.ml.sharded_checkpoint, used by ``-c / --from-checkpoint``) feeds it to torch.device(...) and gets "Invalid device string: 'auto'". 2. device_map='auto' shards the model across every visible GPU. On a multi-GPU host the operator who passed ``-m model -p 8137`` expecting a single-GPU server instead has every GPU on the box commandeered, blocking concurrent training jobs. Replace with ``_default_device()``: probe ``torch.accelerator`` (the accelerator-agnostic API, covers CUDA / XPU / MPS / ROCm) and return ``<acc>:0`` if any device is visible, else "cpu". Single device, real torch.device string, falls back gracefully to CPU on hosts without an accelerator. The hard-coded "cuda:0" of the previous default was also wrong on non-CUDA accelerators. Verified on the CPU dev container: ``forgather inf server -m models/final -c`` (native loader, --from-checkpoint) loads on device=cpu / float32 and /v1/chat/completions returns 20 tokens in 7.5s. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * eval: auto-fall back from --nproc-per-node 'gpu' to 1 on CPU-only hosts Mirror of the train-CLI fix in 3339eb3b. ``forgather eval test`` and the server's enqueue path both hard-coded ``torchrun --standalone --nproc-per-node gpu`` for the ddp / pipeline trainers. torchrun's "gpu" sentinel resolves to the visible CUDA device count, which is 0 on a CPU-only torch build / --gpus none docker / host without an NVIDIA driver -- the launcher aborts with "invalid literal for int() with base 10: 'gpu'" before the eval script starts. When no CUDA device is visible, fall back to nproc-per-node 1 so ``forgather eval test -M MODEL`` keeps working for CPU debugging on the user's existing default (``--trainer ddp``). The CLI emits a one-line warning; the server-side path falls back silently (the scheduler still gates which GPUs are visible via CUDA_VISIBLE_DEVICES, so on GPU servers "gpu" picks the gated subset as before). Verified end-to-end on the CPU dev container against models/final: both ``--trainer simple`` (no torchrun) and ``--trainer ddp`` (torchrun + nproc=1 fallback, gloo backend) return the same eval_loss=1.57 / perplexity=4.80 in ~8-10s wall time. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * webui: allow 0-GPU inference jobs, pin spawned server to -d cpu User report: the Inference modal would not let you submit a job with 0 GPUs (the GPUs field clamped to min=1), so on a CPU-only host the form forced ``requested_gpus=1``, the scheduler couldn't place the job (no GPU available), and the request lingered in the queue forever without spawning anything. Four-part fix end-to-end: 1. webui (InferenceModal.tsx): allow GPUs=0 in the input (min=0, updated clamp, dropped the Math.max(1, ...) baseline for maxGpus), and snap the persisted value into the available range once the GPU list resolves. Adds a "0 = run on CPU" hint when the user picks 0 on a host that has GPUs available, and a "CPU only" suffix when the host has no GPUs at all. 2. tools/forgather_server/routes/queue.py: add ``inference`` to ``_ZERO_GPU_JOB_TYPES``. The route was rejecting ``requested_gpus=0`` for inference jobs with HTTP 400 "requested_gpus must be >= 1 for inference jobs", which is no longer true now that the inference server's _default_device() detects CPU and the launcher pins ``-d cpu`` on zero-GPU dispatches. 3. tools/forgather_server/inference_ops.py: change the ``build_inference_command(device=...)`` default from a hard-coded ``"cuda:0"`` to ``None``, which omits the ``-d`` flag entirely and lets the server's own _default_device() decide. Callers can still pin a specific device when they have one. 4. tools/forgather_server/launcher.py: in spawn_inference_process, compute ``device = "cpu" if not gpu_indices else None`` so a zero-GPU dispatch produces an explicit ``-d cpu`` in the argv (visible in the TTY log / process listing for triage) while GPU dispatches inherit the server's default-device behaviour against the CUDA_VISIBLE_DEVICES the launcher already sets. Verified end-to-end inside the CPU dev container: ``POST /api/queue {requested_gpus: 0, job_type: "inference", ...}`` spawned ``server.py ... -d cpu -m models/final -T bfloat16 -a sdpa -c ...`` against the user's local model, the health endpoint returned healthy, and a chat completion returned 15 tokens. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * inference modal: expose explicit device override (incl. HF auto-shard) Follow-up to 21d44e11. The previous fix let the launcher derive the spawned server's ``-d`` flag from gpu_indices (cpu when 0 reserved, else omit so the server self-detects), but offered no way for the operator to pass anything else from the webui -- in particular the "auto" sentinel that HF's device_map='auto' expects for sharding a model across multiple GPUs (HF loader only, incompatible with ``--from-checkpoint`` because the native loader feeds device strings directly to torch.device()). Thread an optional ``device`` field through job_params: * InferenceModal.tsx: add a Device select next to GPUs with options ``default`` (empty -> launcher decides), ``auto`` (HF device_map='auto'), ``cpu``, ``cuda:0``. Inline-warn when the operator picks ``auto`` while ``--from-checkpoint`` is on, and block the submit since the spawn would crash with "Cannot use 'auto' device with checkpoint loading". * scheduler.py:_build_inference: pull ``device`` from job_params (trimmed, blank -> None) and forward it to spawn_inference_process. * launcher.py:spawn_inference_process: accept an optional ``device`` kwarg. Caller's explicit value (e.g. ``"auto"``) wins; only when it's None do we fall back to the gpu_indices-derived choice (cpu / omit) from the prior commit. Verified end-to-end against ``sshleifer/tiny-gpt2`` on the CPU dev container: enqueue with ``{requested_gpus: 0, device: "auto", from_checkpoint: false}`` -> spawned argv ``... -d auto -m /tmp/tiny-gpt2 -T float32 ...``, /health -> healthy, chat completion returned 5 tokens. (HF's device_map='auto' picks CPU when no GPU is visible, so the path is correct on CPU hosts too.) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * webui: allow 0-GPU eval and training jobs, isolate zero-GPU subprocesses Follow-on to 21d44e11 and fe18c5cd. The same min={1} GPUs lock that blocked CPU-only inference from the webui also blocked eval and training. Five-part fix end-to-end: * routes/queue.py: add ``training`` and ``eval`` to _ZERO_GPU_JOB_TYPES so the POST /api/queue validation no longer rejects requested_gpus=0 for these types. (Now valid because the CLI / launcher paths fall back to nproc=1 on zero-GPU dispatch.) * launcher.py:_spawn_subprocess: when gpu_indices is empty, set CUDA_VISIBLE_DEVICES="" on the subprocess env. Previously the branch was skipped, so a zero-GPU dispatch on a GPU host inherited the parent's (typically unset) CUDA_VISIBLE_DEVICES and torchrun's "gpu" sentinel grabbed every GPU on the box -- defeating the point of reserving 0. Now the subprocess truly sees no CUDA. * launcher.py:build_command (training): accept ``gpu_indices`` and override nproc_per_node="gpu" -> 1 when the reservation is empty in single-node (standalone) mode. Mirrors the train CLI's fix in 3339eb3b. Cluster dispatches still use rdzv_args's cluster_nproc. * eval_ops.py:build_eval_command: take ``gpu_indices`` directly instead of guessing via torch.cuda.is_available() inside the server process. The server-process check was wrong on a GPU host that happened to dispatch a zero-GPU job. spawn_eval_process in launcher.py now passes gpu_indices through. * EvalModal.tsx + SubmitModal.tsx: drop the Math.max(1, ...) baseline for maxGpus, switch input min to 0, fix the onChange clamp, add an inline "0 = run on CPU" hint. EvalModal also gains the useEffect that snaps the persisted value into [0, maxGpus] once the GPU list resolves (mirrors InferenceModal). Verified inside the CPU dev container against the local forgather server: POST /api/queue {requested_gpus: 0, job_type: "eval", ...} -> spawned ``torchrun --standalone --nproc-per-node 1 .../eval_script.py ... --trainer ddp ...`` POST /api/queue {requested_gpus: 0, job_type: "training", ..., config: "v2.yaml"} -> spawned ``torchrun --standalone --nproc-per-node 1 .../train_script.py -p .../tiny_llama ... v2.yaml`` Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * SubmitModal: always clamp requestedGpus to [0, maxGpus] after GPU list loads Follow-up to d8d6dc1e. Even after switching min={0}, the train modal still showed (and submitted) requestedGpus=1 on a 0-GPU host because the seed-from-cache useEffect only set requestedGpus when one of two branches fired (cached override present, or config pinned a fixed worker count). With neither -- the common case for "nproc_per_node: gpu" configs and no prior submit -- the initial useState(1) stuck through to submit. Add a second useEffect that clamps the current value into [0, maxGpus] whenever the GPU list resolves (mirroring EvalModal and InferenceModal). It runs independent of gpusTouched / the seed effect; clamping is idempotent so the ordering doesn't matter, and on a 0-GPU host the form now correctly snaps to 0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * SubmitModal: add nproc override field for CPU debugging User report: training without GPUs only spawns 1 worker process (because nproc_per_node='gpu' falls back to 1 when no GPU is reserved), which underutilises a multi-core CPU. The dynamic-args form can already drive ``nproc_per_node`` for configs that expose it, but that's config-dependent and easy to miss for operators just trying to run a CPU debug pass. Add a dedicated single-node nproc override that's plumbed independently of the config's dynamic args, mirroring the train CLI's ``--nproc`` (3339eb3b): * SubmitModal: text input next to GPUs, with the current config's ``nproc_per_node`` shown as placeholder. Accepts an integer or torchrun's ``gpu`` / ``cpu`` / ``auto`` sentinels. Blank = use the config (and the launcher's "gpu"->1 fallback when 0 GPUs are reserved). Forwarded as ``job_params.nproc`` only when non-empty. * scheduler._build_training: read ``nproc`` from job_params, pass to spawn_training_process as ``nproc_override``. * launcher.spawn_training_process / build_command: accept ``nproc_override`` and use it instead of the config's nproc_per_node. Only applied in single-node (standalone) mode; cluster dispatches still take per-peer nproc from rdzv_args. Verified inside the CPU dev container against the local forgather server: POST /api/queue {requested_gpus: 0, job_params: {nproc: "4"}, ...} -> spawned ``torchrun --standalone --nproc-per-node 4 .../train_script.py ...`` with 4 worker processes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * EvalModal: add nproc override field, mirroring SubmitModal Match the train-modal change in d6c6e8fd. The eval CLI doesn't take ``--nproc`` (eval uses ``--trainer simple/ddp/pipeline`` and the launcher derives nproc from gpu_indices), but for CPU debugging it's useful to spawn more than one ddp worker (the launcher's "gpu"->1 fallback otherwise underutilises a multi-core host). * EvalModal: text input next to GPUs. Disabled for --trainer simple (it bypasses torchrun entirely). Accepts an integer or torchrun's ``gpu`` / ``cpu`` / ``auto`` sentinels. Blank = use the gpu_indices-derived default. Forwarded via job_params.nproc only when non-empty and trainer != simple. * scheduler._build_eval: extract ``nproc`` from job_params (it isn't a flag for eval_script.py so it doesn't belong in the _EVAL_SCRIPT_ARGS passthrough spec) and pass to spawn_eval_process as nproc_override. * launcher.spawn_eval_process + eval_ops.build_eval_command: accept ``nproc_override`` and use it instead of the gpu_indices-derived default. Explicit override wins; the 0-GPU "gpu"->1 fallback is the next-priority default. Verified inside the CPU dev container: POST /api/queue {requested_gpus: 0, job_type: "eval", job_params: {trainer: "ddp", nproc: "3", ...}} -> spawned ``torchrun --standalone --nproc-per-node 3 .../eval_script.py ...`` with 3 worker processes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: address PR #64 findings (CUDA-hide regression, UX clarity, dedup) Acting on the backend / webui reviews of bug/fix-cpu-support. Backend (block-merge candidate from review): * launcher._spawn_subprocess no longer sets CUDA_VISIBLE_DEVICES="" on every empty-gpu_indices dispatch -- that regressed the ConvertModal / FinalizeModal escape hatch where requested_gpus=0 paired with --device cuda:0 means "use host CUDA without a reservation" (ConvertModal even shows an inline hint about this: "0 = CPU only; raise if you set --device cuda…"). Move the isolation into a per-spawn helper _isolate_cuda_extra_env() and invoke it only from spawn_training_process / spawn_eval_process, not from spawn_inference_process (single-process, ``-d`` flag pins the device directly) or any *_ops spawn path that doesn't use torchrun. Operator-supplied CUDA_VISIBLE_DEVICES via extra_env still wins -- setdefault, not unconditional set. Backend (nit-cleanup): * scheduler: factor the duplicated nproc-coercion ladder into a single _coerce_nproc_override(raw) helper used by both _build_training and _build_eval. Backend (observability): * scheduler._build_training / _build_eval: log at INFO when a training / eval job dispatches with 0 GPUs reserved (single-node only -- cluster dispatches don't go through this path). Operators who accidentally enqueue at requested_gpus=0 through a non-modal path get a visible breadcrumb in server.log instead of silently running on CPU. Webui (should-fix from review): * EvalModal nproc placeholder was "auto", but the default the launcher uses for --trainer ddp / pipeline is torchrun's "gpu" sentinel (falls back to 1 on 0-GPU dispatch). Change placeholder to "gpu" so the hint matches reality. * InferenceModal auto+from_checkpoint warning was jargony ("the native loader"). Rewrite in user-visible terms: names HF device_map sharding and the "Load via Forgather checkpoint" toggle by their UI labels. Deferred / declined from review: * distributed.py force-gloo override on CPU even for explicit backend="nccl": keep as-is. CPU + nccl is always a config error; current behaviour logs at INFO when overriding, which is the right loudness for "we substituted a working backend for a broken request". * transformers 5.0-5.8 compat of the create_causal_mask fix: installed in the dev container is 5.9.0; pyproject pins ~=5.0 but the prior commit (0499a06d) already established the forward-compat strategy with the same authors. Will add a version-aware shim if a 5.0-5.4 user reports breakage. * Webui: limited device dropdown (no cuda:1 / xpu:0 / mps:0), free-form nproc validation, title-only help a11y, inline "width: 6em": all legitimate nits, none block CPU-only operation; defer to a separate UX pass. Verified helpers behave as advertised: _coerce_nproc_override(" 4 ") -> "4" _coerce_nproc_override("") -> None _coerce_nproc_override(3) -> "3" _coerce_nproc_override(None) -> None _isolate_cuda_extra_env([], None) -> {"CUDA_VISIBLE_DEVICES": ""} _isolate_cuda_extra_env([0, 1], None) -> None _isolate_cuda_extra_env([], {"CUDA_VISIBLE_DEVICES": "0,1"}) -> {"CUDA_VISIBLE_DEVICES": "0,1"} (operator wins) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Inference client: surface vLLM reasoning trace separately from content (#65) When a vLLM reasoning parser is active (e.g. --reasoning-parser qwen3 on Qwen3.6) the model's pre-answer thinking lands in delta.reasoning / message.reasoning, not delta.content. Both the Python CLI client and the TS webui client were reading only content, so a thinking model that ran out of token budget mid-<think> appeared to return nothing at all. Both clients now consume the reasoning channel: - Python CLI (tools/inference_server/client.py): _consume_chat_stream and _consume_chat_response helpers, shared by get_completion and single_shot, print reasoning in ANSI dim then content normal. ANSI codes are resolved per call (not at import time) so test harnesses that capture stdout still get plain text. A single streaming chunk carrying both reasoning and content is handled in full (no continue-after-reasoning that would silently drop the content). conversation_history stores only content, and an empty assistant turn is never pushed when the budget is exhausted inside <think>. Reads delta.reasoning via getattr + model_extra to stay compatible with older openai-python releases that don't type the field. - TS webui: streamChatCompletion now yields a tagged ChatDelta ({ kind: "content" | "reasoning"; text }); runChatCompletion returns ChatResult ({ content, reasoning? }). streamSse is generic and its contract is documented — extract returning undefined skips the frame, any other value is yielded verbatim. The chat panel accumulates reasoning and content into separate fields on the trailing assistant turn (re-reading via index so future intermediate mutations cannot get overwritten by the spread) and renders reasoning in a dim italic block above the content. Reasoning is a UI-only field on ChatMessage and is stripped before any request body and any persistence write. Interleaved reasoning/content from the model is normalized into two contiguous blobs and rendered reasoning-above-content; parsers that genuinely interleave would need an ordered segment list, which qwen3 does not require in practice. Co-authored-by: jdinalt <dinalt@kitt.dinalt.net> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Add user-added inference server registry to webui (#66) * Add user-added inference server registry to webui The Inference → Model picker listed only spawned/cluster inference jobs; external OpenAI-compatible servers (vLLM, remote inference, a teammate's box) required retyping URL + token every session. Mirror the dataset_server "User-added servers" pattern: a JSON-backed registry at <config>/server/ inference_server_registry.json (0600), CRUD'd via /api/inference-servers/ user, surfaced as a dedicated section in InferenceModelPanel. Token resolution in the inference proxy gains a registry fallback after the existing JobRecord + cluster-inventory lookups, so tokens never round-trip back to the browser for saved entries. Per-entry verify_tls toggle lets operators opt out of TLS chain validation for SSH-tunneled remotes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Disambiguate picker selection + clean up on user-server delete Review caught two real bugs in the new Inference picker: (1) when a user-registered URL coincides with a spawned/cluster server's URL, both rows render selected because the picker compared URL strings — fixed by pinning the last-picked row's {kind,id} in component state and falling back to URL match only when nothing has been picked; (2) after deleting the selected user-server, baseUrl stayed pointing at the orphaned URL — added an effect to clear baseUrl/authToken when the picked user-entry disappears, mirroring DatasetsPanel. Also documented the intentional node-local scope of the inference registry (no cluster wake-up call on mutation, unlike datasets). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Read-only demo mode for public webui exposure (#67) * Add --demo read-only mode for public webui exposure A new --demo CLI flag lets the Forgather server be safely shown to anonymous visitors. The AuthMiddleware blocks every mutating HTTP method outside a narrow allowlist (login/logout, inference-proxy read-throughs) and returns 403; response serializers redact bearer tokens from /api/jobs and /api/services so a curl can't exfiltrate credentials minted for spawned inference / dataset jobs. The webui learns the flag via /api/auth/status, shows an amber DEMO MODE banner at all times, masks token fields, and disables mutation controls on the most user-visible surfaces (Add modals, Jobs cleanup, all .destructive buttons via CSS). Independent of demo mode: the URL + auth-token inputs in the inference panel are now readOnly so they can only be populated by picking a server in the running- / user-registered lists or via the Add modal — manual typing was never a real workflow. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix demo-mode layout: keep sidebar+main side-by-side The .app.demo-mode { flex-direction: column } override collapsed the horizontal layout, stacking the main pane below the sidebar. Instead, reserve 26px of padding-top on .app and absolutely-position the banner into it so the row flex layout is preserved. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Demo-mode 403 must not trigger a forced re-login The webui's fetch wrapper dispatches AUTH_REQUIRED_EVENT on every 401 or 403 that isn't tagged as upstream-auth-failed or proxy-refused, so a demo-mode policy 403 (e.g. clicking a GPU toggle) was bouncing the user back to the login page. Tag the demo-blocked response with X-Forgather-Demo-Blocked: 1 and add the matching skip to the fetch wrapper, following the existing X-Upstream-Auth-Failed pattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Move demo notice to sidebar chip; show Forgather version in header The full-width amber banner was visually loud. Replace it with a compact "DEMO MODE" chip in the sidebar header, next to "Forgather Server". Also surface the server's Forgather package version as a small "v<x.y.z>" label in the same row — visible in both demo and non-demo mode so it's always discoverable. Both are sourced from /api/auth/status so the existing useDemoMode + a new useServerVersion hook share one query. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Add --fs-root allowlist for path-accepting APIs Jupyter-lab-style root restriction: pass --fs-root <path> (repeatable) to scope every read/write that takes a client-supplied filesystem path to descendants of one of the listed roots. Paths outside the allowlist get 403 with X-Forgather-Fs-Root-Denied: 1 (so the webui fetch wrapper doesn't mistake the policy refusal for a session-expiry and bounce to login). In --demo mode the allowlist defaults to {forgather repo root} ∪ {registered search roots} if --fs-root wasn't supplied — that's the curated project content the operator already declared browsable. Without --demo the default stays unrestricted, preserving historical behaviour for local installs. Wired into every path-accepting endpoint: - fs.py: browse/path-exists/mkdir/new-file/rename/copy/move/delete-* (browse also filters out symlink targets that escape the roots, and the parent-link is hidden when it would point above a root) - configs.py: every /config/* and /template/source endpoint - docs.py: /docs/file, /docs/asset - projects.py: /project*, workspace create/init/new-project Frontend: hides the server-config gear in demo mode (the file it opens isn't reachable through fs-root anyway), and disables the scheduler / restart / shutdown gears with explanatory tooltips (backend 403 still backs them up). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Allowlist dataset-server load proxy in demo mode POST /api/dataset-server/proxy/load is a read-through to the upstream dataset_server's /v1/load — body carries a dataset spec, response is the materialized handle the webui then pages through. No local state mutation, so it belongs in the demo allowlist alongside the inference proxy POSTs. Without this, the Datasets panel couldn't open any dataset for browsing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Allowlist cluster dataset-server load proxy in demo mode Add a small templated-pattern matcher to the demo allowlist so /api/cluster/dataset_server_proxy/<server_id>/load — the cluster variant of the dataset-load proxy already on the static list — gets the same read-through treatment. The matcher requires the variable middle segment to be a single path component, so a crafted .../load/.. tunnel can't sneak through. Without this, the Datasets cluster view 403'd when the user opened any dataset for examination via a peer node, while the local-only Datasets view (which hits /api/dataset-server/proxy/load directly) worked fine. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Address PR review: token-leak fixes, fs-root tightening, demo polish Critical / High (token leak surfaces in demo mode): - /api/dataset-servers/local/<id>/bundle now 403s in demo mode (the forgather-dataset:// URI it returns embeds the real bearer token under the key ``bundle``, which the key-name-based redactor can't catch). The webui's "Copy bundle" button is also hidden in demo. - /api/queue _to_model now wraps job_params + dynamic_args through redact_sensitive_in_demo. Inference job_params carry auth_token directly, and extra_env can carry FORGATHER_DATASET_SERVER_TOKEN. - /api/config/dynamic-args now enforces the fs-root allowlist on project_dir (missed in the original sweep — template preprocess follows ``-- include`` chains anywhere project_dir points). - AddInferenceServerModal + AddDatasetServerModal mirror the panel's demo treatment: token field is masked + read-only in demo mode and the Show/Copy buttons are hidden, so a pre-filled value can't be revealed even if the submit button is greyed. - The × remove buttons on user-added server rows pick up the ``destructive`` class so the global demo-mode CSS disables them. Spawned-server TTY leak (new flag): - --quiet-tokens on tools/inference_server/server.py and tools/dataset_server/server.py replaces the bearer-value-bearing launch banner with a one-line "value suppressed" message. Token still works; it's still cached to its per-port file for local client discovery; only the captured TTY pane is sanitized. Wired through inference_ops, dataset_server_ops, launcher.py, scheduler, and surfaced as a checkbox in the Inference and Dataset Server modals (start + create-service flows both pass it through). Medium: - paths.is_path_in_fs_root fails closed on empty/whitespace input (was silently passing when CWD happened to sit inside an fs-root). - configure_fs_roots raises ValueError when the operator supplied --fs-root values but none resolved — refusing to silently fall back to unrestricted. - /api/auth/login removed from the demo allowlist (dead entry; the middleware short-circuits on _OPEN_PATHS before the demo gate). - cluster._forgather_version promoted to a public forgather_version() helper; old name preserved as an alias. - Collapsed sidebar now shows a compact DEMO indicator (was invisible in collapsed state). - "+ Add server" buttons in inference + dataset panels disabled with explanatory tooltip in demo (was a dead-end: modal opened then Add was greyed at the bottom). Lower: - _demo_path_allowed templated-pattern matcher rejects \\\\, \\x00, and % in the variable middle segment (defense vs. non-decoding ASGI servers). - Sidebar demo chip gets role="status" + aria-label for screen readers. - Sidebar version label gets ellipsis + max-width so a long version ("0.1.0+local.abc1234") can't push the gear icons off the edge. - redact_sensitive_in_demo docstring spells out dict/list-only and key-name-based scoping so future callers know what slips through. Docs: - tools/forgather_server/README.md: new --demo and --fs-root rows in the CLI flag table, plus two full sections (Demo mode, Filesystem allowlist, Quiet-tokens) before Authentication overview. The user-facing docs/forgather-server.md is a symlink, so updated. - tools/inference_server/README.md and tools/dataset_server/README.md: added --quiet-tokens documentation in their auth sections. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Round-2 review fixes: fs-root on enqueue + CR/LF + disabled-button CSS Second-pass agent review on commit 5d33317d turned up one HIGH and a few NITs. All addressed. HIGH — Security: - POST /api/queue (enqueue) was calling config_ops.load_dynamic_args on req.project_dir without _enforce_fs_root. Demo mode hides this (POST /api/queue is mutation-blocked anyway), but outside demo with --fs-root the enqueue path follows ``-- include`` chains anywhere project_dir points — bypassing the allowlist. Added the gate at the top of enqueue(). LOW — Security: - _demo_path_allowed templated-pattern middle-segment reject set now also rejects \\r and \\n. Uvicorn already rejects these upstream of ASGI, but the matcher shouldn't depend on the host server's normalization. Comment also flags the inherited "no %, no slashes, no control chars" constraint for future _DEMO_MUTATION_ALLOWLIST_PATTERNS additions. NIT — UI: - Disabled "+ Add server" buttons rendered fully primary-accent — looked live but blocked clicks. The global ``button:disabled`` rule only applied to .destructive / .secondary. Extended it to cover all disabled buttons. Not addressed (intentional): the substring matcher in _SENSITIVE_KEY_FRAGMENTS will false-positive on user-defined keys like ``next_page_token`` or ``token_count`` in dynamic_args. Real concern, but tightening the match (whole-word, exact, etc.) has its own risks (might miss legitimate variants). The redactor's docstring already calls out the key-name-based scoping; further tightening can be a follow-up if a real false-positive turns up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Pre-rendered docs cache + configurable Docs landing page (#68) * docs_build: pre-render mkdocstrings directives for the webui docs viewer The webui's `/api/docs/file` endpoint serves raw markdown, so `:::` mkdocstrings directives in `docs/api/*.md` appear unrendered. This adds a build step that walks `docs/`, expands directives via griffe, and writes a parallel tree at `docs/.built/<rel>.md`. Incremental: a page rebuilds only when its source or any of the Python files it resolved against are newer than the cached output. A `.deps.json` sidecar tracks per-page dependencies relative to the repo root. This commit lands the library only — CLI, server-side fallback, and build-pipeline integration follow in subsequent commits. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs_build: add `forgather docs` CLI Wires the docs_build library into the CLI as `forgather docs build` (plus `forgather docs clean`). `--check` makes it CI-friendly: exits non-zero when any page would be rebuilt without writing anything. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs_build: gitignore docs/.built/ and exclude it from the mkdocs build The previous commit accidentally added `docs/.built/` to the tree; this removes it, gitignores the directory, and adds an `exclude_docs` entry in `mkdocs.yml` so the real mkdocs build doesn't double-render API pages whose `:::` directives mkdocstrings already expands at render time. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * forgather-server: serve pre-rendered docs from docs/.built when available When the docs view fetches a markdown file under <repo>/docs/, prefer docs/.built/<rel>.md if it exists and is not older than the source. Pages outside docs/ (e.g. the symlinked tools/forgather_server/README.md) keep their existing behaviour, and any source edit since the last build falls through to the raw markdown so users never see stale content. The response's reported path is still the canonical source so the frontend resolves relative asset references against the right directory. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * build-webui.sh: pre-render docs cache alongside the webui bundle When the webui static bundle is built, also run `forgather docs build` so the Docs panel can render `:::` API directives in `docs/api/*.md` without falling back to raw markdown. The step is opt-out (SKIP_DOCS_BUILD=1) and best-effo…
jdinalt
added a commit
that referenced
this pull request
Jun 8, 2026
…, reuse) From the high-effort review of this branch: - Event-loop blocking (#1/#2): run_train's advanced commit now awaits asyncio.to_thread(_submit_via_cli) instead of running subprocess.run on the loop; query_model / diloco_control commits and diloco_status / the inference & diloco list handlers are likewise offloaded; _inference/_diloco list_servers probe reachability concurrently (bounded pool) so it's O(timeout) not O(N*timeout). - dynamic_args divergence (#3): the advanced (CLI) path now renders flags via the canonical forgather.cli.diloco._dynamic_cli_from_schema + a new config_ops.load_dynamic_args_schema, so store_true/store_false are honored and match `forgather submit` (was: False bools silently dropped). - Reuse/drift (#6): _parse_dataset_source delegates to the shared CLI parse_dataset_source (fallback kept). - _infer_requested_gpus (#5): coerce a numeric-string nproc_per_node via int() instead of rejecting it (was: silently reserved 1 GPU). - cleanup_jobs (#4): catch non-HTTP errors per queue_id so one OSError doesn't abort the remaining removals. - _DISCLOSURE_NOTE lookup (#8): .get() with an inline fallback (no KeyError if a future mode is added). - tools_authoring docstring (#9): drop the stale propose_new_config_from_template reference. Deferred (documented): routing advanced submit fully in-process instead of shelling out (#1 altitude), and extracting a shared discover/pick helper for the three _*_servers modules (#7). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds a comprehensive testing guide (
docs/development/testing.md) that documents how to run and work with the Forgather test suite. This is intended to help developers understand the test organization, how to run different test categories, and best practices for testing during development.Key Changes
torchrun, including checkpoint integration tests, dataloader dispatcher tests, and pipeline parallelism testsconftest.pypytest.iniNotable Details
torchrunhttps://claude.ai/code/session_017oY6i89L7mUfSTQ75hnra6