Skip to content

Leave loss inputs intact when applying position weights - #8

Merged
davek44 merged 1 commit into
mainfrom
fix-loss-inplace-weights
Oct 15, 2026
Merged

davek44 merged 1 commit into
mainfrom
fix-loss-inplace-weights

Conversation

@davek44

@davek44 davek44 commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

PoissonMultinomialLoss.forward scaled y_true and y_pred by the position weights in place. The trainer computes the loss before updating its metrics (_train_batch_cov, _validate_cov). So the train/valid r, r² and spec it logs, and the stop_stat built from them, were computed on edge-downweighted tensors. This PR makes the multiply out of place. Loss values and gradients are unchanged.

Impact on logged metrics: with weight_range: 10, weight_exp: 8, the edge bins are scaled to 0.1. Down-weighting them inflates valid r by about 0.005 overall, and by up to about 0.015 for broad histone marks (e.g. H3K36me3, H3K4me1). hound_eval scores unweighted tensors, so its numbers never matched the trainer's for the same checkpoint and fold. After this change they do. Metrics logged by earlier poisson_mn runs are position-weighted.

Evidence:

  • For a trained checkpoint, re-running the trainer's validation loop with this loss call reproduces the logged metrics.tsv to 1e-4 in every group. Without the side effect, it matches hound_eval to 1e-4.
  • New test test_position_weights_leave_inputs_intact fails on main and passes here. src/tests/test_loss_weight.py: 34 passed.

🤖 Generated with Claude Code

PoissonMultinomialLoss scaled y_true and y_pred in place, so the trainer's
train/valid r, r2 and spec (and the spec stop_stat) were computed on
edge-downweighted tensors.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@davek44
davek44 merged commit b3b30ae into main Oct 15, 2026
2 of 3 checks passed
@davek44
davek44 deleted the fix-loss-inplace-weights branch October 15, 2026 22:23
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.

1 participant