Skip to content

docs: separate client training from model submission - #304

Merged
umeradl merged 4 commits into
InfiniteZeroFoundation:developfrom
Santiagocetran:docs/client-training-266
Nov 2, 2026
Merged

umeradl merged 4 commits into
InfiniteZeroFoundation:developfrom
Santiagocetran:docs/client-training-266

Conversation

@Santiagocetran

@Santiagocetran Santiagocetran commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #266.

Corrects the technical client-service guide and reference-service descriptions to match the current workflow: train-lms saves a local model artifact; submit-lm uploads it to IPFS and submits its CID on-chain.

Updates the entrypoint name to train_client_model and explains worker paths and artifact selection. Runtime behavior and differential-privacy logic are unchanged.

Validation:

  • Executable Python AST unchanged.
  • Function signature, manifest entry and CLI examples checked against source.
  • All 11 relative links resolve; git diff --check passes.

Integration note: will need reconcilitaion after #292 lands. Preserve its DP changes alongside these wording corrections.

@umeradl

umeradl commented Oct 30, 2026

Copy link
Copy Markdown
Member

Reviewed against develop in an isolated worktree (branch applies cleanly on top of current tip e9424af, which is also the merge-base; 1 commit, no conflicts — confirmed by both git merge-tree --write-tree locally and GitHub's own mergeable: MERGEABLE / mergeStateStatus: BLOCKED, where BLOCKED is only the missing approval).

CI on edfc0e0: CI OK green — Docs covers the relative links, and Python covers the unchanged service tests. This is a docs and comments PR, so the local work was checking each description against the source it describes.

Claimed: executable Python is unchanged in cache_model_0/services/client.py.

Verified — exact: I parsed both versions with ast, removed the module and function docstrings, and compared ast.dump: equal. The only remaining differences are comments, which ast drops.

Claimed: the entrypoint is train_client_model, matching the manifest and the CLI.

Verified — correct: cache_model_0/manifest.json:107 has the key "train_client_model". In dincli/cli/client.py, train-lms calls require_manifest_key("train_client_model") and writes "function_name": "train_client_model" into the worker job. The signature block in the guide matches the def line by line.

Claimed: train-lms stages inputs, runs the service in a worker, and ends with a local file; submit-lm uploads and submits on-chain.

Verified — each step against the source:

  • Staging: train-lms calls ensure_file_exists for the client service, the model architecture, models/genesis_model.pth, and models/gm_{GI-1}.pt only when current_GI > 1 and the T2 CID is non-zero. That matches "(when available after GI 1)".
  • Worker mounts: run_worker_container mounts the model dir as /din/model:ro, and each writable_subdirs entry (here models/clients/<address>) as :rw on top (dincli/cli/worker.py:147, :153). That matches "mounted read-only, with the client's model output subdirectory mounted read-write".
  • Worker result: dincli/docker/worker/worker.py defines class WorkerRuntime and writes {"status": "ok", "result": …} to /din/output/result.json. The host checks status != "ok" and maps the path back with relative_to("/din/model").
  • Submission: submit-lm resolves the file with get_local_model_path(…, resolve_dp_config(manifest)) (lm_{gi}.pth or lm_{gi}_{mechanism}.pth, matching the new Output Contract list) and sends taskAuditor.submitLocalModel(cid, ref_gi).

Claimed: train-lms requires the current GI; submit-lm defaults to it and also accepts an earlier one.

Verified — correct: train-lms uses validate_gi_ET_curr_GI (exits unless gi == curr_gi), and submit-lm uses validate_gi_LTE_curr_GI (None → current; rejects 0 and anything above current).

Claimed: the service no longer downloads from IPFS; it reads staged files.

Verified — correct: the service reads model_base_dir/models/genesis_model.pth and gm_{gi-1}.pt and raises FileNotFoundError if they're missing. genesis_model_ipfs_hash is only printed, which matches the new "identifies … staged by the host CLI" wording.

Claimed: all 11 relative links resolve.

Verified — by CI: the Docs job is green on edfc0e0.

One gap, not a blocker for this PR's scope: the old name and the "trains and uploads" wording are still in two other files under Documentation/:

  • Documentation/public/services.md:129, heading 3.1. train_client_model_and_upload_to_ipfs(...) with "Trains the client model on local data and uploads it to IPFS", plus the manifest example at :149. That's the public page model owners read, and it links to the guide this PR fixes.
  • Documentation/technical/manifest.md:73 (manifest example) and :119 ("dincli reads train_client_model_and_upload_to_ipfs from the manifest").

The PR body flags these as out of scope, and issue No. 266 names only "the guide and service comments", so Closes #266 is accurate. But Documentation/ is meant to match develop, and once No. 266 closes nothing tracks these lines. Either fix them at merge (four lines, same wording as this PR) or open a follow-up issue before merging. See the merge-proposal table.

Merge order with PR No. 292: confirmed. A git merge-tree of this branch against PR No. 292's head conflicts in Documentation/technical/services/clients.md. Its Developer/BACK_LOG.md conflict comes from No. 292 being behind develop, not from this PR. Whichever lands second has to keep No. 292's update_laplace text alongside this PR's "save locally" wording.

Not independently re-verified: an end-to-end train-lms → submit-lm run in Docker against anvil/IPFS. No runtime code changed (AST-equal), and every described step was traced in the source above.


Every checkable claim held up. Mergeable as-is. I'd prefer the two stale files above get fixed at merge, so closing No. 266 doesn't leave the public page saying the opposite of the guide.

@umeradl

umeradl commented Oct 30, 2026

Copy link
Copy Markdown
Member

Files changed (2) — as of edfc0e0 (PR head)

Diffed against merge-base e9424af (develop). develop hasn't moved since the branch was cut. GitHub agrees: mergeable: MERGEABLE, mergeStateStatus: BLOCKED (approval only). Confirmed with a local git merge-tree --write-tree dry run: clean. Open PR No. 292 conflicts with this PR in Documentation/technical/services/clients.md (see below).

Documentation/technical/services/clients.md

Field Value
Change Modified
Lines +41/-23
Diff (what exactly is in this PR) Rewrites the Overview, the "Its job is to" list, the Flow summary (now seven steps, from train-lms through the worker to submit-lm, plus a two-command example and the --gi rules), the Inputs table (adds model_base_dir; runtime is now WorkerRuntime), the file-layout note (/din/model, read-only with a read-write output subdir), the training/DP steps ("save … locally" instead of "upload"), and the Output Contract (the two returned path shapes). Renames the entrypoint to train_client_model.
Functionality — how & why How: describes the real split. train-lms stages the service, architecture, genesis model and (after GI 1) the previous global model, runs the worker, and prints the local path. submit-lm picks lm_{gi}.pth / lm_{gi}_{mechanism}.pth from the manifest's DP config, uploads it, and calls submitLocalModel. Why: issue No. 266. The guide described the old fused "train and upload" function, which no longer exists, so a reader would expect train-lms to submit.
Diff vs current develop HEAD None — untouched since merge-base.
Recommended merge proposal Merged as-is. Every step was traced to the source (see the verification comment); links are green in CI Docs.
Actual merge proposal Soon
Pending proposal Merge order with PR No. 292 (update_laplace), which edits the DP sections of this file. Whichever lands second must keep both changes: No. 292's mechanism text and this PR's "save … locally" wording. Not a blocker for this PR.
Local merge conflict No (against develop); Yes against PR No. 292's head
GitHub merge conflict No

cache_model_0/services/client.py

Field Value
Change Modified
Lines +13/-7
Diff (what exactly is in this PR) The module docstring, the train_client_model docstring and two inline comments now say "load staged inputs, save, return the path" instead of "upload".
Functionality — how & why How: no runtime change. The AST with docstrings stripped is equal to develop's. Why: the service's own description still said it uploads (issue No. 266, second bullet).
Diff vs current develop HEAD None — untouched since merge-base.
Recommended merge proposal Merged as-is. AST-equal to develop apart from docstrings. Note that PR No. 292 also edits this file; git merge-tree merges it cleanly.
Actual merge proposal Soon
Pending proposal None
Local merge conflict No
GitHub merge conflict No

Not in this PR: Documentation/public/services.md, Documentation/technical/manifest.md

Field Value
Change None (still stale)
Lines —
Diff (what exactly is in this PR) Untouched. services.md:129 heading train_client_model_and_upload_to_ipfs(...) + "uploads it to IPFS", :149 manifest key; manifest.md:73 manifest key, :119 "dincli reads train_client_model_and_upload_to_ipfs".
Functionality — how & why How: no code reads these names; cache_model_0/manifest.json uses train_client_model. Why it matters: Documentation/ describes develop, and the public page links to the guide this PR corrects, so the two would disagree.
Diff vs current develop HEAD —
Recommended merge proposal Either fix at merge (rename to train_client_model and change "uploads it to IPFS" to "saves it locally; submit-lm uploads it") or open a follow-up issue before merging, because Closes #266 would otherwise leave them untracked.
Actual merge proposal Soon
Pending proposal The merger chooses: fix at merge or open a follow-up issue.
Local merge conflict No
GitHub merge conflict No

Verification

CI on edfc0e0: CI OK green. Locally: an AST comparison of client.py against develop (equal), and every new sentence in the guide traced to dincli/cli/client.py, dincli/cli/worker.py, dincli/docker/worker/worker.py, dincli/cli/context.py and cache_model_0/manifest.json. Full detail is in the verification comment above.

Local vs. GitHub agree: both clean against develop.

umeradl
umeradl previously approved these changes Oct 30, 2026

@umeradl umeradl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved at edfc0e0: client.py is unchanged apart from docstrings (AST comparison); every step in the guide was traced to dincli/cli/client.py, worker.py and the worker runner. Before or at merge: fix the stale train_client_model_and_upload_to_ipfs references in public/services.md and technical/manifest.md, or open a follow-up issue.

Santiagocetran commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator Author

Thanks for reviewing this, @umeradl, and for catching the remaining stale references!

This follow-up includes the public/manifest examples that the original PR description listed as out of scope. Pushed the documentation cleanup in 45b490cc277b50f7f8a70c19805c9da257cc5096. Both public/services.md and technical/manifest.md now use train_client_model and explain that training saves a local artifact, while submit-lm separately selects, uploads, and submits it. The Returns row also correctly describes a local file path.

No runtime or DP behavior changed. All 292 inline relative documentation links resolve, and the executable Python AST remains unchanged. If PR No. 292 lands first, I'll reconcile the overlap while preserving its DP changes and calibration qualifications. Fresh CI and re-review of the new head are still needed.

@robertocarlous

Copy link
Copy Markdown
Collaborator

Reviewed against develop in an isolated worktree (pr-304-review on top of af79a8c9, the PR head). Peer check: OK (Santiagocetran is the author; operator is not).

CI on af79a8c9: CI OK ✅, Python ✅, Solidity ✅, Docs ✅ — all green. This PR is documentation and comment-only; the CI evidence covers the doc-link check and the unchanged Python/Solidity paths.

The PR's own work is two commits: edfc0e0a (clients.md + client.py) and 45b490cc (services.md + manifest.md). The remaining diff against upstream/develop (task.py, test_register_request_cid.py, workflow files, BACK_LOG.md, etc.) comes from upstream merge commits already on develop and is not part of this PR.


Claimed — train_client_model is the correct function name and manifest entry. Verified by direct inspection:

  • cache_model_0/services/client.py L463: def train_client_model(...)
  • cache_model_0/manifest.json L107: "train_client_model": { "type": "custom", ...}
  • dincli/cli/client.py L127: manifest = runtime.require_manifest_key("train_client_model"); L197: "function_name": "train_client_model"

The old name train_client_model_and_upload_to_ipfs no longer exists anywhere in the codebase. The docs now match the real name.

Claimed — client.py changes are doc/comment-only; executable AST unchanged. Verified: all changed lines in cache_model_0/services/client.py are either module-level docstring bullet points or the train_client_model function docstring. No def, return, if, assignment, or call site was touched.

Claimed — all relative links resolve. Verified: python3 .github/scripts/check_doc_links.py Documentation reports "checked 292 inline relative link(s) — all inline relative links resolve." The new link added in manifest.md (dincli/docker/worker/worker.py) points to a file that exists at that path.

Claimed — dincli client train-lms / submit-lm two-command split described accurately. Verified against dincli/cli/client.py: train-lms builds the worker job and returns the local artifact path; submit-lm independently selects the file by GI, wallet, and DP settings and submits on-chain. The doc description of the split is accurate.

Claimed — PR 292 reconciliation noted. Verified: the PR body flags "will need reconciliation after No. 292 lands." The current clients.md in this branch does not include PR 292's update_laplace section (PR 292 is still open). The wording corrections here (post_training_gaussian/post_training_laplace experimental labels, output-contract section, etc.) are present and correct against the current develop state; PR 292's author will need to preserve them on rebase.


No findings. Merge-ready; branch needs update-branch before merge (BEHIND develop, no conflict).

@robertocarlous

Copy link
Copy Markdown
Collaborator

Head at review: af79a8c9
Merge base (upstream develop): behind but clean — no conflict (merge-tree exit 0)


Documentation/technical/services/clients.md

Key Value
Change +64/-30 lines: Overview rewritten for train/submit split; flow summary extended to 7 steps covering worker path; inputs table adds model_base_dir; training path step 1 and step 7 corrected; post_training_gaussian and post_training_laplace headings updated with experimental labels (Track A); output contract section updated with specific file paths; noise-calibration note updated
Pending proposal None — accurate, consistent with source; PR 292's update_laplace section (not yet merged) will need to be preserved on rebase
Recommended merge proposal Merge as-is
Actual merge proposal Soon

cache_model_0/services/client.py

Key Value
Change +18/-12 lines: module-level docstring and train_client_model function docstring updated to reflect train/submit split; no executable logic changed
Pending proposal None
Recommended merge proposal Merge as-is
Actual merge proposal Soon

Documentation/public/services.md

Key Value
Change +16/-14 lines: section 3.1 renamed from train_client_model_and_upload_to_ipfs to train_client_model; description, parameter table and Returns row updated; manifest example entry name corrected
Pending proposal None
Recommended merge proposal Merge as-is
Actual merge proposal Soon

Documentation/technical/manifest.md

Key Value
Change +14/-9 lines: "client manifest-loading path" section adds worker.py link; high-level flow extended to 7 steps; example entry name corrected from train_client_model_and_upload_to_ipfs to train_client_model; custom-service flow section rewritten as 7 steps covering worker lifecycle
Pending proposal None
Recommended merge proposal Merge as-is
Actual merge proposal Soon

robertocarlous
robertocarlous approved these changes Oct 31, 2026 •
@umeradl
umeradl added this pull request to the merge queue Nov 2, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 2, 2026
@umeradl
umeradl added this pull request to the merge queue Nov 2, 2026
Merged via the queue into InfiniteZeroFoundation:develop with commit 1263921 Nov 2, 2026
4 checks passed
@umeradl

umeradl commented Nov 2, 2026

Copy link
Copy Markdown
Member

Actual outcome — PR No. 304 merged as-is through the merge queue (second attempt)

This replaces the predictions in Robbert's pre-merge review comment above with what actually happened when the PR landed.

One commit on origin/develop:

  1. 12639212 — the merge of this PR (a merge commit, not a squash), path A. @robertocarlous approved head 2c463e18, and I queued it. The queue merged it at 2026-11-02 18:01 UTC with parents 8b46e4e0 (develop) and 2c463e18 (PR head). There were no conflicts, as predicted. GitHub shows the PR as MERGED.

First attempt, dropped by the queue (not this PR's fault): the first queue run, on top of PR No. 254 (run 41475597415), failed one Solidity test. The cause was a DEPLOYMENTS_NETWORK race that PR No. 254 introduced; this PR touches no Solidity. The queue removed the PR and left develop untouched. PR No. 311 fixed the race, and the PR was re-queued unchanged, with the same head and the same approval.

Files unchanged from the PR (4 of 4)

Documentation/public/services.md, Documentation/technical/manifest.md, Documentation/technical/services/clients.md and cache_model_0/services/client.py landed exactly as reviewed, with no deviation commit.

Verification

  • Merge-queue CI on gh-readonly-queue/develop/pr-304-8b46e4e0…: run 41495472820, green.
  • develop push CI on 12639212: run 41496595238, green (Docs, Python, Solidity, CI OK).

Local vs. GitHub agree: yes. The merge was clean, as both predicted.

@umeradl umeradl added the merged label Nov 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update the client-service guide to explain training and uploading separately

3 participants