Repository navigation
feat(007): trained inference path for seed-morph78 - #106
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e028323fb1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| raise InferModelError("family probabilities do not match families") | ||
| best = max(range(len(families)), key=lambda i: family_probs[i]) | ||
| vector_blob = ",".join(f"{v:.6f}" for v in vector).encode("utf-8") | ||
| unbind_ok = bool(atoms) and all(a["filler_pred"] == a["filler"] for a in atoms) |
There was a problem hiding this comment.
Include role predictions in the unbind result
When the filler head predicts every token correctly but the role head predicts any wrong positional role, this expression still reports unbind_ok: true. Spec 007 defines unbinding as role-and-filler exactness, so downstream consumers can accept a structurally incorrect result; require both role_pred == role and filler_pred == filler for every atom.
Useful? React with 👍 / 👎.
| weight = model_dir / WEIGHT_FILE | ||
| if not weight.is_file(): | ||
| raise InferModelError(f"missing {weight}") |
There was a problem hiding this comment.
Load the supported heads.pt checkpoint format
When safetensors is unavailable or saving it fails, save_heads intentionally produces heads.pt, and the checkpoint contract declares both formats valid. This hardcoded lookup therefore rejects a valid output from the repository's own trainer as missing; inference should select and load either artifact, as the existing trunk-forward evaluator does.
Useful? React with 👍 / 👎.
| offs = _offsets(tok, text) | ||
| atoms = [] | ||
| for i, token in enumerate(tokens): | ||
| idxs = pool_indices(states.size(0), atom_token_index(text, token, offs)) |
There was a problem hiding this comment.
Align each atom to its actual occurrence
For repeated tokens such as go go, or a token that first appears as a substring such as the a in cat a, atom_token_index ultimately uses text.find(token) and resolves the first matching substring every time. Multiple positional atoms consequently pool the same or an unrelated hidden state while being labeled as different pos_i roles, corrupting their role and filler predictions; use the character span of the enumerated whitespace occurrence instead.
Useful? React with 👍 / 👎.
| tokens = text.split() | ||
| with torch.no_grad(): | ||
| enc = tok([text], truncation=True, max_length=MAX_LEN, return_tensors="pt") |
There was a problem hiding this comment.
Do not emit predictions for truncated-away atoms
For inputs exceeding the tokenizer's MAX_LEN, tokens retains every whitespace token while the encoder truncates the sequence. Atoms beyond the retained offset mapping then get no matching indices and pool_indices silently substitutes token state 1, so the packet fabricates predictions for text the model never encoded and can even report a false successful unbind; reject, omit, or explicitly mark those truncated atoms unsuccessful.
Useful? React with 👍 / 👎.
Same change as #105, rebuilt on
mainafter #104 was squash-merged. #105 had adjacent-insertion conflicts in CHANGELOG, ROADMAP and similar files that the API can't resolve with a merge commit. The file tree is byte-identical to the tested commit.See #105 for the full description, Spark receipt, findings and validation (747 passed; mkdocs strict PASS). Supersedes #105.