fixes from_model function and adds tests (#921)

* fixes from_model function and adds tests

* removes mock accelerator from tests

* fixes tests by adding model_configs where needed

* Apply suggestion from @Copilot

Co-authored-by: Copilot <[email protected]>

* fixes from review

* fixes from review

* fixes from review

* fixes from review

---------

Co-authored-by: Copilot <[email protected]>

Author: NathanHB

src/lighteval/models/transformers/transformers_model.py

@@ -43,7 +43,6 @@
 )
 from transformers.generation.configuration_utils import GenerationConfig
 from transformers.generation.utils import GenerateOutput
-from transformers.models.auto.modeling_auto import MODEL_FOR_CAUSAL_LM_MAPPING_NAMES
 
 from lighteval.data import GenerativeTaskDataset, LoglikelihoodDataset
 from lighteval.models.abstract_model import LightevalModel, ModelConfig
@@ -245,39 +244,34 @@ def cleanup(self):
     @classmethod
     def from_model(
         cls,
-        model: Union[AutoModelForCausalLM, LightevalModel],
-        config: TransformersModelConfig = None,
-        accelerator: "Accelerator" = None,
-        tokenizer_name: str = None,  # custom tokenizer
-        trust_remote_code: bool = False,
-        add_special_tokens: bool = True,
-        skip_special_tokens: bool = True,
-        pairwise_tokenization: bool = False,
-        multichoice_continuations_start_space: bool = None,
-    ):
-        # Slightly hackish way to test if the model is a AutoModelForCausalLM, since the instances don't
-        # derive from this class explicitely
-        assert isinstance(model, LightevalModel) or type(model).__name__ in MODEL_FOR_CAUSAL_LM_MAPPING_NAMES.values()
-
-        if isinstance(model, LightevalModel):
-            return model
+        model: AutoModelForCausalLM,
+        config: TransformersModelConfig,
+        accelerator: Accelerator | None = None,
+    ) -> "TransformersModel":
+        if config is None:
+            raise ValueError("Config must be provided to initialize the TransformersModel via `from_model` method.")
 
         # Instanciate the object without using __init__
         self = cls.__new__(cls)
+
         self.transformers_config = model.config
-        if isinstance(model, TransformersModel):
-            self.config = model.config
-        else:
-            self.config = (
-                config if config is not None else TransformersModelConfig(model_name=model.config.name_or_path)
-            )
-        if config is not None:
-            self.generation_config_dict = config.generation_parameters.to_transformers_dict()
+
+        self.config = config
+        self.multichoice_continuations_start_space = config.multichoice_continuations_start_space
+        self._add_special_tokens = config.add_special_tokens
+        self.skip_special_tokens = config.skip_special_tokens
+        self.pairwise_tokenization = config.pairwise_tokenization
+        self.batch_size = config.batch_size
+        self.continuous_batching = config.continuous_batching
+        self.generation_config_dict = config.generation_parameters.to_transformers_dict()
+
+        self.model_name = config.model_name
+        self.model_sha = config.get_model_sha()
         self._max_length = self._init_max_length()
         self._tokenizer = self._create_auto_tokenizer()
-        self.batch_size = getattr(config, "batch_size", None)
-        self.model_name = _simplify_name(model.name_or_path)
-        self.model_sha = self.config.get_model_sha()
+        self.use_chat_template = uses_chat_template(
+            tokenizer=self._tokenizer, override_chat_template=config.override_chat_template
+        )
 
         # If model_parallel is not set we compare the number of processes with the number of GPUs
         self.model = model
@@ -291,16 +285,6 @@ def from_model(
         else:
             self._device = self.config.device
 
-        self.use_chat_template = uses_chat_template(
-            tokenizer=self._tokenizer, override_chat_template=config.override_chat_template
-        )
-        self._add_special_tokens = add_special_tokens if add_special_tokens is not None else False
-        self.skip_special_tokens = skip_special_tokens if skip_special_tokens is not None else True
-        self.pairwise_tokenization = pairwise_tokenization
-        self.multichoice_continuations_start_space = multichoice_continuations_start_space
-
-        self.precision = _get_dtype(model.dtype, config=self.transformers_config)
-
         if is_accelerate_available():
             model_size, _ = calculate_maximum_sizes(self.model)
             model_size = convert_bytes(model_size)

src/lighteval/pipeline.py

@@ -35,6 +35,7 @@
 
 from lighteval.logging.evaluation_tracker import EvaluationTracker
 from lighteval.metrics import apply_metric
+from lighteval.models.abstract_model import LightevalModel, ModelConfig
 from lighteval.models.model_loader import TransformersModel, load_model
 from lighteval.models.model_output import (
     ModelResponse,
@@ -155,7 +156,7 @@ def __init__(
         tasks: str,
         pipeline_parameters: PipelineParameters,
         evaluation_tracker: EvaluationTracker,
-        model_config=None,
+        model_config: ModelConfig | None = None,
         model=None,
         metric_options=None,
     ):
@@ -205,7 +206,24 @@ def _init_parallelism_manager(self):
 
     def _init_model(self, model_config, model):
         logger.info("--- LOADING MODEL ---")
-        if model_config is not None:
+
+        if model is not None and model_config is not None:
+            if isinstance(model, LightevalModel):
+                raise ValueError(
+                    "You are trying to provide both a LightevalModel and a model config. Please provide only one of them."
+                )
+            return TransformersModel.from_model(
+                model=model,
+                config=model_config,
+                accelerator=self.accelerator,
+            )
+
+        elif model is not None:
+            if isinstance(model, LightevalModel):
+                return model
+            raise ValueError("If not providing a model_config, you need to provide a Lighteval model.")
+
+        elif model_config is not None:
             if self.parallel_context:
                 return NanotronLightevalModel(
                     checkpoint_path=os.path.dirname(self.pipeline_parameters.nanotron_checkpoint_path)
@@ -218,13 +236,6 @@ def _init_model(self, model_config, model):
                 )
             else:
                 return load_model(config=model_config)
-        if isinstance(model, TransformersModel):
-            return model
-        else:
-            return TransformersModel.from_model(
-                model=model,
-                accelerator=self.accelerator,
-            )
 
     def _init_tasks_and_requests(self, tasks: str):
         with local_ranks_zero_first() if self.launcher_type == ParallelismManager.NANOTRON else nullcontext():

tests/models/test_transformers_model.py

@@ -117,6 +117,54 @@ def test_model_creation_model(self):
         self.assertEqual(str(self.model.model), str(self.reference_model))
 
 
+class TestTransformersModelCreationFromModel(unittest.TestCase):
+    def setUp(self):
+        """Set up shared model instance for all tests."""
+        self.reference_model = AutoModelForCausalLM.from_pretrained("gpt2")
+        self.reference_tokenizer = AutoTokenizer.from_pretrained("gpt2")
+
+        max_length = 1234
+        self.reference_tokenizer.model_max_length = max_length
+
+        self.config = TransformersModelConfig(model_name="gpt2", max_length=max_length)
+
+        # Create full model instance
+        self.model = TransformersModel.from_model(
+            model=self.reference_model,
+            config=self.config,
+        )
+
+    def test_model_creation_tokenizer(self):
+        for attribute in [
+            "name_or_path",
+            "vocab_size",
+            "model_max_length",
+            "is_fast",
+            "clean_up_tokenization_spaces",
+            "added_tokens_decoder",
+        ]:
+            with self.subTest(attribute=attribute):
+                self.assertEqual(
+                    getattr(self.model.tokenizer, attribute), getattr(self.reference_tokenizer, attribute)
+                )
+
+    def test_model_creation_attributes(self):
+        """Test that TransformersModel creates and initializes basic attributes correctly."""
+        # Test attributes are set correctly
+        self.assertEqual(self.model.config, self.config)
+        self.assertEqual(self.model.multichoice_continuations_start_space, None)
+        self.assertTrue(self.model._add_special_tokens)
+        self.assertFalse(self.model.pairwise_tokenization)
+        self.assertIsNone(self.model.batch_size)
+        self.assertFalse(self.model.continuous_batching)
+        self.assertEqual(self.model.model_name, self.config.model_name)
+        self.assertEqual(self.model.max_length, self.config.max_length)
+
+    def test_model_creation_model(self):
+        # We can't compare objects directly
+        self.assertEqual(str(self.model.model), str(self.reference_model))
+
+
 class TestTransformersModelProcessing(unittest.TestCase):
     @patch("lighteval.models.transformers.transformers_model.Accelerator")
     def setUp(self, mock_accelerator):