Skip to content

Fix Cohere compatibility logit scaling - #1727

Merged
jlarson4 merged 3 commits into
TransformerLensOrg:devfrom
emerardd:fix/cohere-logit-scale-forward
Sep 4, 2026
Merged

Fix Cohere compatibility logit scaling#1727
jlarson4 merged 3 commits into
TransformerLensOrg:devfrom
emerardd:fix/cohere-logit-scale-forward

Conversation

@emerardd

Copy link
Copy Markdown
Contributor

Description

Fixes #1726

Cohere and Cohere2 weight processing folds cfg.logit_scale into the live unembedding weights, but the Hugging Face model forward still applied its model-level scale afterward. As a result, the first valid compatibility-mode conversion produced logits with the scale applied twice.

This change adds an architecture post-install lifecycle hook. The Cohere adapter uses it to neutralize the Hugging Face model-level logit_scale only after the scaled unembedding weights have been installed. The Bridge config retains the original scale as metadata, and tied input embeddings remain unscaled.

Network-free tiny-config regressions cover both direct process_weights() and the first enable_compatibility_mode() call.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Validation

  • tests/integration/model_bridge/test_cohere_adapter.py: 24 passed
  • Cohere/Cohere2 adapter unit surface: 74 passed
  • Weight-processing unit surface: 27 passed, 1 skipped
  • Full non-slow unit surface: 5,435 passed; the sole nonzero result was a native-Windows GBK decode error in a source-roster test
  • The exact roster test passed after enabling Python UTF-8 mode: 1 passed
  • mypy .: Success, no issues found in 392 source files
  • pycln, pinned isort, Black, and git diff --check: passed

Checklist:

  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (no public documentation change is required; the internal lifecycle hook is documented in code)
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have not rewritten tests relating to key interfaces which would affect backward compatibility

@jlarson4 jlarson4 linked an issue Aug 31, 2026 that may be closed by this pull request
1 task

@jlarson4 jlarson4 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.

Thanks for documenting this issue and putting together the fix @emerardd! Just a note on the CI failure, that is a testing issue that is currently being resolved in #1730, and should be ready soon. A couple small comments on this fix, let me know if you have any questions

"""
return state_dict

def postprocess_weights(self, bridge: Any) -> None:

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.

supported_architectures/AGENTS.md "When to override preprocess_weights" tells adapter authors to fold any config knob HF applies in forward, with Cohere's logit_scale as its worked example and a copy-paste skeleton. Following it produces exactly this bug, because it never says the forward-time factor also has to be taken out.

Please add postprocess_weights to that section: when the factor lives outside the module the bridge wraps, folding it is only half the change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 1544c4ce. I expanded the preprocess_weights guidance in supported_architectures/AGENTS.md to cover the paired post-install lifecycle: when HF applies a factor outside the wrapped component, adapters now track whether the fold occurred and use postprocess_weights() to neutralize the outer runtime factor after installation. The guidance and skeleton also call out that a missing runtime attribute means there is no outer factor to neutralize.

return

model = getattr(bridge, "original_model", None)
if model is None or not hasattr(model, "logit_scale"):

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.

If original_model has no logit_scale, its forward isn't applying one, so the fold alone is already correct and there is nothing to neutralize. A Cohere-architecture module in that shape processes cleanly on dev and hard-fails here. This occurs after the folded weights are installed and _weights_processed is set, so a caller that catches the error is left holding the mis-scaled bridge.

Return instead of raising when the attribute is absent.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 1544c4ce. postprocess_weights() now neutralizes model.logit_scale only when the wrapped model exposes it, and always clears the pending fold state without raising when the attribute is absent. I added a network-free regression that removes the model-level attribute, runs process_weights(), and verifies that processing completes, the attribute remains absent, the unembed weight is folded by 0.5, and cfg.logit_scale remains 0.5. Focused validation passed: 3 end-to-end regressions, 74 Cohere/Cohere2 adapter tests, 27 weight-processing tests with 1 skip, formatting, and mypy over 392 source files.

@jlarson4
jlarson4 merged commit fe0af9c into TransformerLensOrg:dev Sep 4, 2026
25 checks passed
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 Report] Cohere compatibility mode double-applies logit_scale in forward

2 participants