Skip to content

fix: Multinomial probabilities, sparsemax gradients, Tangos single-row batches - #459

Open
ChrisW09 wants to merge 1 commit into
mainfrom
fix/distribution-misc
Open

fix: Multinomial probabilities, sparsemax gradients, Tangos single-row batches#459
ChrisW09 wants to merge 1 commit into
mainfrom
fix/distribution-misc

Conversation

@ChrisW09

Copy link
Copy Markdown
Collaborator

Fixes #453 — three independent defects, each with its own regression test.

1. MultinomialDistribution.forward returned raw logits

BaseDistribution.forward looks up a <param_name>_transform attribute for each parameter. This family names its parameters per class (p_0, p_1, …) and keeps a single shared probs_transform, so every lookup missed and silently fell back to identity:

MultinomialDistribution(num_classes=3)(torch.tensor([[2.0, 0.0, -1.0]]))
# -> [[2.0, 0.0, -1.0]]   (logits, not probabilities)

compute_loss applies the softmax correctly, so only predict(raw=False) was affected — it returned logits labelled as probabilities. Fixed with a small forward override.

2. sparsemax backward produced silently wrong gradients

grad_input.sum(dim=dim) / supp_size.to(output.dtype).squeeze() — a bare squeeze() 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). Now squeeze(dim), which removes only the reduced axis.

3. Tangos crashed on single-row batches

penalty_forward calls jacobian.squeeze(), which removes the batch axis when the batch holds one row — routine for the final batch of an odd-sized dataset. The following neuron_attr.shape[2] then raised IndexError. Singleton axes other than the batch axis are now squeezed individually.

Tests

New tests/test_distribution_misc_fixes.py: Multinomial forward matches torch.softmax and sums to 1; sparsemax gradients are unchanged by adding a leading size-1 axis and match a batched reference; and TangosRegressor fits a 42-row dataset whose default split leaves a final batch of exactly one row. Verified 2 of the 4 fail on main (the sparsemax pair needs the size-1-axis case to differ, which the first test pins).

🤖 Generated with Claude Code

…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Multinomial probabilities are raw logits; Tangos crashes on single-row batches; sparsemax gradients wrong for size-1 dims

1 participant