fix: Multinomial probabilities, sparsemax gradients, Tangos single-row batches - #459
Open
ChrisW09 wants to merge 1 commit into
Open
fix: Multinomial probabilities, sparsemax gradients, Tangos single-row batches#459ChrisW09 wants to merge 1 commit into
ChrisW09 wants to merge 1 commit into
Conversation
…w batches Three independent defects: - MultinomialDistribution.forward returned raw logits. BaseDistribution .forward looks up a '<param_name>_transform' attribute per parameter, but this family names its parameters per class (p_0, p_1, ...) and keeps a single shared 'probs_transform', so every lookup missed and fell back to identity. predict(raw=False) therefore returned logits labelled as probabilities (e.g. [2.0, 0.0, -1.0]) while compute_loss applied the softmax correctly. Overridden to apply the shared transform. - SparsemaxFunction.backward divided by supp_size.squeeze(), and a bare squeeze() also drops unrelated size-1 axes; the resulting shape mismatch broadcast into silently wrong gradients whenever a non-reduced dimension had size 1 (e.g. NODE with depth 1). Now squeeze(dim), which only removes the reduced axis. - Tangos.penalty_forward called jacobian.squeeze(), which removes the batch axis when the batch holds a single row -- routine for the final batch of an odd-sized dataset -- so the following neuron_attr.shape[2] raised IndexError. Singleton axes other than the batch axis are now squeezed individually. Fixes #453 Co-Authored-By: Claude Fable 5 <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
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.
Fixes #453 — three independent defects, each with its own regression test.
1.
MultinomialDistribution.forwardreturned raw logitsBaseDistribution.forwardlooks up a<param_name>_transformattribute for each parameter. This family names its parameters per class (p_0,p_1, …) and keeps a single sharedprobs_transform, so every lookup missed and silently fell back to identity:compute_lossapplies the softmax correctly, so onlypredict(raw=False)was affected — it returned logits labelled as probabilities. Fixed with a smallforwardoverride.2.
sparsemaxbackward produced silently wrong gradientsgrad_input.sum(dim=dim) / supp_size.to(output.dtype).squeeze()— a baresqueeze()also drops unrelated size-1 axes, and the resulting shape mismatch broadcasts into wrong gradient values whenever a non-reduced dimension has size 1 (e.g. NODE with depth 1). Nowsqueeze(dim), which removes only the reduced axis.3. Tangos crashed on single-row batches
penalty_forwardcallsjacobian.squeeze(), which removes the batch axis when the batch holds one row — routine for the final batch of an odd-sized dataset. The followingneuron_attr.shape[2]then raisedIndexError. Singleton axes other than the batch axis are now squeezed individually.Tests
New
tests/test_distribution_misc_fixes.py: Multinomialforwardmatchestorch.softmaxand sums to 1; sparsemax gradients are unchanged by adding a leading size-1 axis and match a batched reference; andTangosRegressorfits a 42-row dataset whose default split leaves a final batch of exactly one row. Verified 2 of the 4 fail onmain(the sparsemax pair needs the size-1-axis case to differ, which the first test pins).🤖 Generated with Claude Code