[QEff. Finetune]: Adding base class and HF class #658

quic-swatia · 2025-12-09T07:30:34Z

Added Base Model class and HF model class.
Base Model class will support FT for any custom model and will be a common skeleton for any model, including HF.
Added unit tests for these.

Signed-off-by: Swati Allabadi <[email protected]>

quic-meetkuma

Please use your reference code wisely. The implementation lacks main functionalities. The tests are not extensive and implemented in naive manner. Please correct them as well.

Reference code: https://github.com/quic-meetkuma/LightningLLMs/blob/hf_trainer/LightningLLM/components/model.py

quic-meetkuma · 2025-12-16T07:20:19Z

QEfficient/finetune/experimental/core/model.py

+        logger.info(f"Model {self.model_name} does not expose input embeddings", logging.WARNING)
+        return None
+
+    def resize_token_embeddings(self, new_num_tokens: int) -> None:


This is applicable for NLP models. We can't put it in BaseClass.

Replied above.

This method looks specific to NLP. I dont find reference of this method in VIT or Whisper model of HF. I think the else case is taking care of it. So it is fine for now.

quic-meetkuma · 2025-12-16T07:20:46Z

QEfficient/finetune/experimental/core/model.py

+    def forward(self, *args, **kwargs):
+        return self.model(*args, **kwargs)
+
+    def get_input_embeddings(self):


This is applicable for NLP models. We can't put it in BaseClass.

Replied above.

I am a bit doubtful on this. But looking at most of the HF model's implementation (e.g. ASR and Vision) this method exists. So it is fine.

Bdw do we need this method at all? I think we can get rid of it as it is not even used.

quic-meetkuma · 2025-12-16T07:22:07Z

QEfficient/finetune/experimental/core/model.py

+        return self._model
+
+    @property
+    def tokenizer(self) -> Any:


This is applicable for NLP models. We can't put it in BaseClass. Better to create an abstract method called "preprocessor" which defines generic preprocessing function applicable for the model. There wont be any implementation here but the children class should implement that. In case of LLMs, this method should return tokenizer.

quic-meetkuma · 2025-12-16T07:22:31Z

QEfficient/finetune/experimental/core/model.py

+        """Create and return the underlying torch.nn.Module."""
+        ...
+
+    def load_tokenizer(self) -> Any:


Specific to LLMs. We can not put it here.

Replied above

What I feel is that the naming should be "load_preprocessor" instead of load_tokenizer. In the HFModel class's load_preprocessor, you should actually load the tokenizer and return it.

Tokenizer is one of the preprocessor. Other models can have different preprocessor.

quic-meetkuma · 2025-12-16T07:22:54Z

QEfficient/finetune/experimental/core/model.py

+    @abstractmethod
+    def load_model(self) -> nn.Module:
+        """Create and return the underlying torch.nn.Module."""
+        ...


use "pass" as it is explicit.

quic-meetkuma · 2025-12-16T07:50:58Z

QEfficient/finetune/experimental/tests/test_model.py

+
+    # get_input_embeddings: underlying model lacks method, should warn and return None
+    with mock.patch("QEfficient.finetune.experimental.core.model.logger.info") as mocked_log:
+        assert m.get_input_embeddings() is None


create a proper dummy model which returns some embeddings rather than None. Your test should use HFModel class instead of some dummy class.

quic-meetkuma · 2025-12-16T07:52:50Z

QEfficient/finetune/experimental/tests/test_model.py

+        raising=False,
+    )
+
+    m = HFModel.create("hf-name")


No need to call individual class's create method. There is a reason to use a registry functionality.

Purpose of create is different than component_registry. Explained in another comment.

Move it to ComponentFactory and then instantiate from there.

quic-meetkuma · 2025-12-16T07:54:13Z

QEfficient/finetune/experimental/tests/test_model.py

+    tok = m.load_tokenizer()
+
+    # tokenizer was loaded and pad token inserted
+    model.AutoTokenizer.from_pretrained.assert_called_once_with("hf-name")


Dont write such tests filled with assert_called_once etc.. We have written such tests in past but that was not an appropriate thing. It was a makeshift arrangement because of monolith structure of code. Write extensive and proper tests. If a function has made some changes to the model's structure then use that to test rather then counting how many times the function gets called.

quic-meetkuma · 2025-12-16T08:06:16Z

QEfficient/finetune/experimental/tests/test_model.py

+    def load_tokenizer(self):
+        return "dummy-tokenizer"
+
+


Where is PEFT related test cases?

There is no PEFT code, hence no test cases.

Replied above for PEFT code.

We need to add test code for the PEFT models. Add it in the BaseClass, make an instance of HFModel with PEFT config and check whether the model is modified with PEFT changes or not.

quic-meetkuma · 2025-12-16T08:07:53Z

QEfficient/finetune/experimental/core/model.py

+
+
+@registry.model("hf")
+class HFModel(BaseModel):


We need to load the model based on configuration as well. That is mainly for testing purpose. In integration tests we will not load an entire model consists of 32 layers. But we will only load the same model with 2 or 4 layers and do further testing. For that purpose config should be used to load the model. Check huggingface documentation on how to do that.

Adding base class and Hf class

f88f01d

Signed-off-by: Swati Allabadi <[email protected]>

quic-swatia requested review from ochougul, quic-amitraj, quic-hemagnih and quic-rishinr as code owners December 9, 2025 07:30

quic-swatia requested review from quic-akuruvil, quic-meetkuma and vbaddi December 10, 2025 21:28

Adding unit test cases

defab15

Signed-off-by: Swati Allabadi <[email protected]>

quic-swatia force-pushed the model_classes_exp branch from a663197 to defab15 Compare December 10, 2025 21:36

quic-meetkuma requested changes Dec 16, 2025

View reviewed changes

[QEff. Finetune]: Adding base class and HF class #658

Are you sure you want to change the base?

[QEff. Finetune]: Adding base class and HF class #658

Conversation

quic-swatia commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

quic-meetkuma left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

quic-swatia commented Dec 9, 2025 •

edited

Loading