Skip to content

Fix CohereASR training loss double shift#46958

Open
w-Jessamine wants to merge 1 commit into
huggingface:mainfrom
w-Jessamine:fix/cohere-asr-loss-shift
Open

Fix CohereASR training loss double shift#46958
w-Jessamine wants to merge 1 commit into
huggingface:mainfrom
w-Jessamine:fix/cohere-asr-loss-shift

Conversation

@w-Jessamine

@w-Jessamine w-Jessamine commented Jun 29, 2026

Copy link
Copy Markdown

CI

Summary

Fixes #46894.
Supersedes #46947, which was closed after the fork branch was renamed.

CohereASR right-shifts labels into decoder_input_ids, then used self.loss_function (ForCausalLMLoss) to compute the training loss. That loss shifts labels internally again, so training targets were effectively labels[..., 1:].

This switches CohereASR to a plain CrossEntropyLoss, matching the Moonshine fix and seq2seq models that already align decoder inputs from labels before computing loss.

Tests

  • PYTHONPATH=src /tmp/transformers-pr-venv/bin/python -m pytest tests/models/cohere_asr/test_modeling_cohere_asr.py::CohereAsrModelTest::test_training_loss_no_double_shift -q

CI recap

Dashboard: View test results in Grafana
Latest run: 28370058252
Result: success | Grafana metrics are not available yet.

@github-actions

Copy link
Copy Markdown
Contributor

[For maintainers] Suggested jobs to run (before merge)

run-slow: cohere_asr

@eustlb eustlb left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#47090 is the correct fix IMO, see #47090 (review) for explaination
cc @vasqu @SunMarc if you agree

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.

CohereASR training-loss double-shift bug (same pattern as Moonshine #46784)

2 participants