Repository navigation
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. Pages changed in this PR: |
mangeshraut712
left a comment
There was a problem hiding this comment.
Nice work: the packed KL + optional RPS objective is carefully written, decide() now forces return_dict=True and reads .logits so it keeps working with JevDecisionOutput, and the independent Categorical/RPS reference test (ragged questions, ignore index, soft labels, weights, confident fp32 gradients, microbatch accumulation) is excellent.
A few residual issues:
-
Soft labels are documented to "sum to one" but never checked.
JevForDecisionLosswill happily take a soft row that sums to 0.5 or 1.5;kl_divwill still run and RPS CDFs will be biased. A cheaptargets.sum(-1)check on the active slots (or a test that this is rejected / renormalized) would match the docstring. -
num_options == 0/ empty packed logits.positions = torch.arange(num_options.max())andscores.argmax(-1)are undefined or awkward on an empty question. Even a single-question test with zero options (or an emptylogits) would show whether this should raise a clear error. -
if rps_weight:treats a programmed0.0correctly, but a negative weight would still be applied. If the weight is user-facing onJevConfig, a< 0guard might be worth it. -
Shape of
logits. The loss indexeslogitsas 1-D (logits[indices],logits.numel()). The new output type says shape(total_num_options,), which matches the currentforward. If a caller ever passes a batched(1, total)tensor,numel()and advanced indexing will go wrong. Worth assertinglogits.ndim == 1(or flattening a leading batch-1 dim) so a future wrapper cannot fail silently.
accepts_loss_kwargs = False is a good call given packed soft labels, and normalizing by valid-question count even when loss_weights is set is consistent with the new docs.
ArthurZucker
left a comment
There was a problem hiding this comment.
NiceQ Smalls nits but will be nice
| """ | ||
| decision_config = decision_config or self.decision_config | ||
| inputs = {"input_ids": input_ids, **kwargs} | ||
| inputs = {"input_ids": input_ids, **kwargs, "return_dict": True} |
There was a problem hiding this comment.
maybe return tuple decortor missing?
|
[For maintainers] Suggested jobs to run (before merge) run-slow: jev |
What does this PR do?
JEV currently returns only logits, so training needs a custom loss outside the model. This adds
JevForDecisionLossthroughLOSS_MAPPINGand returns standardloss/logitsoutputs for Trainer and TRL, while preserving decision-pipeline inference.The default follows the AutoTrust JEV-9B model card: KL(target || model) over active options, plus 0.5 × RPS for Score questions. Includes hard/soft targets, question weights, training docs, and loss/gradient tests. The loss uses max-excluded
log1pnormalization to retain confident-prediction gradients.Extracted from #50411 against
main; that PR stays a draft.Validation: 20 tests passed, 4 skipped, 31 subtests passed; 128 independent float64 objective/gradient comparisons passed.
make stylepasses.make check-repoonly reports the existingtorch.distributed.get_ranktype-check warning, also reproduced on unchangedmain.Who can review?
Anyone interested in decision-model training.