fix: make Casper training path actually learn - #5
Merged
Merged
Conversation
|
Deployment failed for project casper with the following error: Learn More: https://vercel.com/gratechs-projects?upgradeToPro=build-rate-limit |
There was a problem hiding this comment.
🔵 Needs a closer look
It introduces a large new core training implementation and optimizer/update behavior that needs careful human validation for correctness and performance impact.
Pull request overview
This PR repairs and expands the NIYAH C training path so it can learn end-to-end (beyond LM-head-only updates), and wires the new full-parameter training implementation into the trainer binary, self-checks, build script, and CI.
Changes:
- Adds deterministic non-zero initialization plus a new full-parameter, truncated-BPTT training step implementation (detached-KV boundary).
- Updates the trainer CLI to use live tokenizer-derived vocabulary size, run full-parameter training steps, and emit training metadata.
- Extends the NIYAH self-check and CI smoke to include a regression that requires loss reduction and a backbone (attention) weight change, plus adds a debug sanitizer smoke job.
File summaries
| File | Description |
|---|---|
scripts/build.sh |
Links the new full trainer implementation into niyah and trainer, and includes it in the lint gate. |
README.md |
Updates documentation to reflect the new full-parameter training path and detached-KV training description. |
Core_CPP/niyah_train.c |
Switches trainer to tokenizer-derived vocab size, deterministic init, and full-parameter training step. |
Core_CPP/niyah_train_full.h |
Introduces the public API for deterministic init + full-parameter training step. |
Core_CPP/niyah_train_full.c |
Implements deterministic initialization and full-parameter detached-KV training, including optimizer update. |
Core_CPP/niyah_main.c |
Adds a self-check regression that enforces loss reduction and attention-backbone weight updates. |
.github/workflows/ci.yml |
Adds a debug sanitizer smoke build/run alongside existing release smoke coverage. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+282
to
+286
| ALLOC_FLOATS(grad, nw); | ||
| ALLOC_FLOATS(x_in, (size_t)m->cfg.n_layers * d); | ||
| ALLOC_FLOATS(att_norm, (size_t)m->cfg.n_layers * d); | ||
| ALLOC_FLOATS(q, (size_t)m->cfg.n_layers * d); | ||
| ALLOC_FLOATS(k, (size_t)m->cfg.n_layers * kd); |
Comment on lines
+20
to
+22
| * Updates token embeddings, every attention/FFN projection, RMSNorm scales, | ||
| * and the LM head with AdamW. The causal KV cache is a deliberate truncated | ||
| * backpropagation boundary: a token receives gradients through its own Q/K/V |
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
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.
Clean rebase onto current main 7da5605.
Repairs the C training path that previously combined zero-initialized weights with output-head-only updates and an invalid token/context check.
Changes:
Supersedes stale PR #4.