Fix typing hints for different rope parameters per layer type #43320
Conversation
Signed-off-by: Tcc0403 <[email protected]>
|
[For maintainers] Suggested jobs to run (before merge) run-slow: gemma3, gemma3n, modernbert, modernbert_decoder |
Tcc0403
force-pushed
the
special-rope-params-typehint
branch
from
b2609cd to
f94f8a0
Compare
Signed-off-by: Tcc0403 <[email protected]>
Tcc0403
force-pushed
the
special-rope-params-typehint
branch
from
f94f8a0 to
ae430d1
Compare
|
View the CircleCI Test Summary for this PR: https://huggingface.co/spaces/transformers-community/circle-ci-viz?pr=43320&sha=ae430d |
zucchini-nlp
left a comment
zucchini-nlp
left a comment
There was a problem hiding this comment.
Hey, when you are ready, please mark PR as "ready for review". I just left couple comments
| self.rope_parameters = self.rope_parameters if self.rope_parameters is not None else default_rope_params | ||
| if ( | ||
| self.rope_parameters.get("sliding_attention") is not None | ||
| and self.rope_parameters.get("full_attention") is not None | ||
| ): | ||
| self.rope_parameters = default_rope_params |
There was a problem hiding this comment.
not really correct because only one key could be present technically, if the config.layers_types consists of the same type, for ex all layers are sliding. This happens in tests and Ig those a few might fail now
There was a problem hiding this comment.
if that's the case, should we also avoid direct access to both keys?
self.rope_parameters["full_attention"].setdefault(
"rope_theta", kwargs.pop("rope_theta", self.default_theta["global"])
)
self.rope_parameters["sliding_attention"].setdefault(
"rope_theta", kwargs.pop("rope_local_base_freq", self.default_theta["local"])
)| attn_logit_softcapping: float | None = None, | ||
| rope_parameters: RopeParameters | dict[str, RopeParameters] | None = None, | ||
| rope_parameters: dict[Literal["full_attention", "sliding_attention"], RopeParameters] | None = None, |
There was a problem hiding this comment.
can you apply these diffs in modular files and then run make fix-repo? We keep the model code in modular, which generates the rest of the files automatically
2 participants
What does this PR do?
Fixes #43316
gemma3/gemma3nandmodernbert/modernbert_decoderrequire different rope parameters per layer type, which expectsrope_parametersto be a dictionary with two keyssliding_attentionandfull_attentionmapping toRopeParameterscontainingrope_type.This PR updates the typing hints for these models and sets the new format dectection stricter.
Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.
@zucchini-nlp