馃毃 Remap indexers layer_type - #48974
Conversation
| # Remap layer types if needed | ||
| if hasattr(self, "layer_types"): | ||
| self.layer_types = remap_legacy_layer_types(self.layer_types) | ||
| if hasattr(self, "mtp_layer_types"): | ||
| self.mtp_layer_types = remap_legacy_layer_types(self.mtp_layer_types) |
There was a problem hiding this comment.
Before this, the remap was happening only manually in a few given configs... Much better to make it automatic in the general __post_init__ of all, since it's only an extremely cheap lookup table
|
run-slow: axk2, deepseek_v32, glm5_next, glm_moe_dsa, granitemoehybrid, hy_v4, nemotron_h, olmo_hybrid, qwen3_5, qwen3_5_moe, qwen3_next, qwen4_exp |
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
c92b546 to
e6dcc5e
Compare
vasqu
left a comment
vasqu
left a comment
There was a problem hiding this comment.
Let's mark with 馃毃 just in case since it's technically breaking if anyone relied on the exact namings of our cache (which shouldnt be many imo)
| else: | ||
| self.layer_types = remap_legacy_layer_types(self.layer_types) |
There was a problem hiding this comment.
ah ok so this is now a general remap in config utils?
|
|
||
| # Mapping from old names to new names | ||
| _LEGACY_LAYER_TYPE_REMAP = { | ||
| "conv": "linear_attention", # only in LFMv2 |
There was a problem hiding this comment.
Because it's part of the names we ALLOW 馃ゲ So I have no idea why it was in the deprecated mapping
| # correct conventions), so it avoids trying to reset it | ||
| if (remapped := remap_legacy_layer_types(self.layer_types)) != self.layer_types: | ||
| self.layer_types = remapped | ||
| if getattr(self, "mtp_layer_types", None) is not None: |
There was a problem hiding this comment.
now needed for the dsa rename ig?
| # This check should not be needed, but sometimes `layer_types` is a read-only @attribute (already following | ||
| # correct conventions), so it avoids trying to reset it | ||
| if (remapped := remap_legacy_layer_types(self.layer_types)) != self.layer_types: | ||
| self.layer_types = remapped |
There was a problem hiding this comment.
can we not just pass the layer type optionally so it has these conditional in there?
There was a problem hiding this comment.
Well we could pass the config into remap_legacy_layer_types directly to do it there, but then we have to also pass a list[str] to say which attributes we want to check (here both layer_types and mtp_layer_types) which makes it clunky IMO
There was a problem hiding this comment.
Or we decide to do it for both when the config is explicitly given
There was a problem hiding this comment.
Actually let me do that
|
[For maintainers] Suggested jobs to run (before merge) run-slow: axk2, deepseek_v32, glm5_next, glm_moe_dsa, granitemoehybrid, hy_v4, nemotron_h, olmo_hybrid, qwen3_5, qwen3_5_moe, qwen3_next, qwen4_exp |
CI recapDashboard: View test results in Grafana |
3 participants
What does this PR do?
As per the title. Instead of
deepseek_sparse_attention,qwen_sparse_attention,xxx_sparse_attentionetc etc, let's remap to a simplerindexed_attentionas the cache/mask will always be similar for those variantsAlso, much better to ALWAYS call
remap_legacy_layer_typesin the general__post_init__vs only a few given Configs, since it's only a cheap lookup table -> will avoid many issues for custom checkpoints/configs/finetunes etc