Adding support for Nandi Models#45101
Conversation
Co-authored-by: Vishesht27 vishesht27@gmail.com
|
We are the team at RTA AI Labs, a tiny but passionate startup dedicated to making high-performance language models more accessible through efficient architecture. Today, we are excited (and a little nervous!) to submit this PR to add support for Nandi, our custom "smol" model series. As a very small team, we have poured our hearts, late nights, and limited resources into building Nandi. We believe that the future of AI belongs to efficient, edge-compatible models, and we’ve designed Nandi to punch significantly above its weight class in terms of reasoning and throughput. Bringing Nandi to the Hugging Face ecosystem is a massive milestone for us. It is the "make or break" step for our upcoming release, as it will allow the community to easily fine-tune, deploy, and experiment with what we've built. Why this PR matters
A small note to the maintainers |
|
Heya super excited to see this 👋 Just as a heads up, we have holidays + torch conference so reviews will be delayed for at least a week ish. Not sure if I will be the one reviewing or someone else, but as a first step it would be best to fully utilize modular https://huggingface.co/docs/transformers/v5.5.0/en/modular_transformers#implementing-a-modular-file Appreciate the work, and dont hesitate to ping us 😄 |
|
Hi @vasqu, thanks for the heads up! I’ve updated the PR to fully utilise the modular transformer format as suggested in the documentation. Looking forward to your feedback whenever you're back. |
xenova
left a comment
There was a problem hiding this comment.
Love seeing smaller models! Just an FYI before the main reviewers get to this... your usage of modular is not correct, as I see many classes which are mostly duplicates of existing implementations. Take a look at how a modular file like https://github.com/huggingface/transformers/blob/def5e6864fe4f2bbd7f056f37366f4dd0d693097/src/transformers/models/apertus/modular_apertus.py is laid out.
e.g.,
class ApertusRMSNorm(LlamaRMSNorm):
pass
class ApertusRotaryEmbedding(LlamaRotaryEmbedding):
passare valid usages of modular.
| config.num_attention_heads * self.head_dim, config.hidden_size, bias=config.attention_bias | ||
| ) | ||
|
|
||
| @deprecate_kwarg("past_key_value", new_name="past_key_values", version="4.58") |
There was a problem hiding this comment.
no need to deprecate here. I don't think v5 shouldn't have these decorators anymore :)
| return f"{tuple(self.weight.shape)}, eps={self.variance_epsilon}" | ||
|
|
||
|
|
||
| class NandiRotaryEmbedding(nn.Module): |
There was a problem hiding this comment.
identical to normal LlamaRotaryEmbedding afaict.
| config.num_attention_heads * self.head_dim, config.hidden_size, bias=config.attention_bias | ||
| ) | ||
|
|
||
| @deprecate_kwarg("past_key_value", new_name="past_key_values", version="4.58") |
| self.input_layernorm = NandiRMSNorm(config.hidden_size, eps=config.rms_norm_eps) | ||
| self.post_attention_layernorm = NandiRMSNorm(config.hidden_size, eps=config.rms_norm_eps) | ||
|
|
||
| @deprecate_kwarg("past_key_value", new_name="past_key_values", version="4.58") |
|
Hi @xenova , I’ve inherited the Llama modules wherever necessary and followed the modular structure closely. I also removed the deprecate_kwargs decorator. Please let me know if there are any other changes you’d like me to make. |
|
hi @xenova , we have made all changes from our side, can you review and please give us some feedback |
|
Sorry this PR got a bit lost in my notifications, will try to take a look next week. Could you sync with main in the mean time and fix whatever broke 🤗 |
vasqu
left a comment
There was a problem hiding this comment.
Some initial comments from my side 🤗 sorry for all the delays. Imo, the biggest point is the virtual cache layers. This should be changed tbh just for the sake to avoid adding more cache layer types and staying consistent with our APIs
What should change
- Num hidden layers is indicating the total number of layers with repetition
- No virtual cache layer
- Passing the offset directly as kwarg instead which will be added on read time within the attention module
This way we keep compatibility with everything that we already have without adding too much custom stuff
There was a problem hiding this comment.
You are missing your docs for the model under docs/source/en/model_doc/... (and to be added to the _toctree); maybe you can take a look at recent model additions that use the New Model label 🤗
| ) | ||
|
|
||
|
|
||
| @strict(accept_kwargs=True) |
There was a problem hiding this comment.
| @strict(accept_kwargs=True) | |
| @auto_docstring(checkpoint="FrontiersMind/Nandi-Mini-150M-Instruct") | |
| @strict |
see e.g. llama here
transformers/src/transformers/models/llama/configuration_llama.py
Lines 29 to 30 in d63bb4a
|
|
||
|
|
||
| @strict(accept_kwargs=True) | ||
| class NandiConfig(PretrainedConfig): |
There was a problem hiding this comment.
| class NandiConfig(PretrainedConfig): | |
| class NandiConfig(LlamaConfig): |
could we inherit from Llama for some values at least? I think that would avoid a few attributes and you would only need to override those that differ in values
If you dont need something from llama, you can do attr = AttributeError() to avoid inheriting it
| if not self.layer_sharing: | ||
| self.layer_sharing_repeats = 1 | ||
|
|
||
| if self.factorized_embedding and self.embedding_rank <= 0: |
There was a problem hiding this comment.
Wouldn't it make sense to bound this to only the embedding rank and factorize in any case when this is >0?
| if self.hidden_size % self.num_attention_heads != 0: | ||
| raise ValueError( | ||
| f"`hidden_size` ({self.hidden_size}) must be divisible by `num_attention_heads` ({self.num_attention_heads})." | ||
| ) | ||
| if self.layer_sharing_repeats < 1: | ||
| raise ValueError(f"`layer_sharing_repeats` must be >= 1, got {self.layer_sharing_repeats}.") |
There was a problem hiding this comment.
Anything that validates if the attributes have sane values should be in
| for decoder_layer in self.layers[: self.config.num_hidden_layers]: | ||
| for repeat_idx in range(repeats): | ||
| # Each repeat gets its own virtual cache slots offset by num_hidden_layers, | ||
| # so repeat 0 uses slots 0..N-1 and repeat 1 uses slots N..2N-1, etc. | ||
| repeat_cache = ( | ||
| _VirtualLayerCache(past_key_values, repeat_idx * self.config.num_hidden_layers) | ||
| if (past_key_values is not None and repeat_idx > 0) | ||
| else past_key_values | ||
| ) | ||
| hidden_states = decoder_layer( | ||
| hidden_states, | ||
| attention_mask=causal_mask, | ||
| position_embeddings=position_embeddings, | ||
| position_ids=position_ids, | ||
| past_key_values=repeat_cache, | ||
| use_cache=use_cache, | ||
| **kwargs, | ||
| ) |
There was a problem hiding this comment.
| for decoder_layer in self.layers[: self.config.num_hidden_layers]: | |
| for repeat_idx in range(repeats): | |
| # Each repeat gets its own virtual cache slots offset by num_hidden_layers, | |
| # so repeat 0 uses slots 0..N-1 and repeat 1 uses slots N..2N-1, etc. | |
| repeat_cache = ( | |
| _VirtualLayerCache(past_key_values, repeat_idx * self.config.num_hidden_layers) | |
| if (past_key_values is not None and repeat_idx > 0) | |
| else past_key_values | |
| ) | |
| hidden_states = decoder_layer( | |
| hidden_states, | |
| attention_mask=causal_mask, | |
| position_embeddings=position_embeddings, | |
| position_ids=position_ids, | |
| past_key_values=repeat_cache, | |
| use_cache=use_cache, | |
| **kwargs, | |
| ) | |
| # We go through each layer x times instead of once like usually | |
| for decoder_layer in self.layers[: self.actual_layers]: | |
| for _ in range(self.repeated_layers): | |
| hidden_states = decoder_layer( | |
| hidden_states, | |
| attention_mask=causal_mask, | |
| position_embeddings=position_embeddings, | |
| position_ids=position_ids, | |
| past_key_values=past_key_values, | |
| use_cache=use_cache, | |
| **kwargs, | |
| ) |
Adjusted here then
There was a problem hiding this comment.
Maybe we need to pass the repeat factor in here to shift the layer idx access then within the attention module --> guess we do end up to something similar you initially did but with a passed arg/kwarg
| _tied_weights_keys = {"lm_head.weight": "model.embed_tokens.weight"} | ||
| _tp_plan = {"lm_head": "colwise_gather_output"} | ||
| _pp_plan = { | ||
| "lm_head_proj": (["hidden_states"], ["hidden_states"]), | ||
| "lm_head": (["hidden_states"], ["logits"]), | ||
| } | ||
|
|
||
| def __init__(self, config): | ||
| super().__init__(config) | ||
| self.model = NandiModel(config) | ||
| self.vocab_size = config.vocab_size | ||
|
|
||
| self.lm_head_proj = ( | ||
| nn.Linear(config.hidden_size, config.embedding_rank, bias=False) if config.factorized_embedding else None | ||
| ) | ||
| self.lm_head = nn.Linear( | ||
| config.embedding_rank if config.factorized_embedding else config.hidden_size, | ||
| config.vocab_size, | ||
| bias=False, | ||
| ) | ||
|
|
||
| self.post_init() |
There was a problem hiding this comment.
| _tied_weights_keys = {"lm_head.weight": "model.embed_tokens.weight"} | |
| _tp_plan = {"lm_head": "colwise_gather_output"} | |
| _pp_plan = { | |
| "lm_head_proj": (["hidden_states"], ["hidden_states"]), | |
| "lm_head": (["hidden_states"], ["logits"]), | |
| } | |
| def __init__(self, config): | |
| super().__init__(config) | |
| self.model = NandiModel(config) | |
| self.vocab_size = config.vocab_size | |
| self.lm_head_proj = ( | |
| nn.Linear(config.hidden_size, config.embedding_rank, bias=False) if config.factorized_embedding else None | |
| ) | |
| self.lm_head = nn.Linear( | |
| config.embedding_rank if config.factorized_embedding else config.hidden_size, | |
| config.vocab_size, | |
| bias=False, | |
| ) | |
| self.post_init() | |
| def __init__(self, config): | |
| super().__init__(config) | |
| self.lm_head_proj = ( | |
| nn.Linear(config.hidden_size, config.embedding_rank, bias=False) if config.factorized_embedding else nn.Identity() | |
| ) |
you dont need to override what is already in the parent
| if self.lm_head_proj is not None: | ||
| hidden_states = self.lm_head_proj(hidden_states) |
There was a problem hiding this comment.
| if self.lm_head_proj is not None: | |
| hidden_states = self.lm_head_proj(hidden_states) | |
| hidden_states = self.lm_head_proj(hidden_states) |
There was a problem hiding this comment.
Could you add some tests for the tokenizer? This file should be empty tbh
|
[For maintainers] Suggested jobs to run (before merge) run-slow: auto, nandi |
|
View the CircleCI Test Summary for this PR: https://huggingface.co/spaces/transformers-community/circle-ci-viz?pr=45101&sha=00c441 |
Co-authored-by: Vishesht27
This PR adds support for codes for the upcoming Nandi series models. We also appreciate the valuable feedback and thorough review provided by @vasqu and @ArthurZucker 🤗🙏