Homogeneize generation params #428

clefourrier · 2024-12-09T14:31:13Z

This PR does 3 things:

Provide an homogeneized API for people to use to provide model generation parameters in model configs. Those parameters are notably provided to all models which can take them (vllm, open ai, tgi, transformers, ...)
Renames BaseModel to TransformersModel
Also allows TransformersModels to use a transformers.GenerationConfig object directly, when created programmatically

I would put system_prompt, fewshot_seeds, and use_chat_template in the GenerationParameters too since they are generation parameters logically, but it can be another PR

Closes #16

HuggingFaceDocBuilderDev · 2024-12-09T14:33:13Z

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.

…n the models to use other attributes later

clefourrier · 2024-12-11T18:09:33Z

Interestingly, using an explicit generation config seems to mess up this quite a lot

…ams set by default which slow down generations

…ammatically if needed

albertvillanova

Great! I only reviewed it partially. Some comments below.

src/lighteval/models/transformers/transformers_model.py

src/lighteval/main_endpoint.py

Co-authored-by: Nathan Habib <[email protected]> Co-authored-by: Albert Villanova del Moral <[email protected]>

src/lighteval/models/vllm/vllm_model.py

src/lighteval/models/transformers/transformers_model.py

Co-authored-by: Nathan Habib <[email protected]>

…ia the generationparams instead

Co-authored-by: Albert Villanova del Moral <[email protected]>

NathanHB

nice to have a coherent config, few nits and good to go

NathanHB · 2024-12-18T14:45:38Z

src/lighteval/models/vllm/vllm_model.py

@@ -117,6 +122,7 @@ def __init__(
        self.precision = _get_dtype(config.dtype, config=self._config)

        self.model_info = ModelInfo(model_name=self.model_name, model_sha=self.model_sha)
+        self.sampling_params = SamplingParams(**config.sampling_params)


where does the config.sampling_params come from ? I do not see them in VLLMModelConfig

after testing it indeed does not run

NathanHB · 2024-12-18T14:46:29Z

src/lighteval/models/transformers/transformers_model.py

+    def __post_init__(self):
+        super()
+
+        logger.warning(


Suggested change

logger.warning(

warnings.warn(

NathanHB · 2024-12-18T14:47:07Z

src/lighteval/models/transformers/transformers_model.py

+        super()
+
+        logger.warning(
+            "Careful, the BaseModel name is deprecated and will be removed, you should use TransformersModel instead!"


Suggested change

"Careful, the BaseModel name is deprecated and will be removed, you should use TransformersModel instead!"

"Careful, the BaseModel name is deprecated and will be removed, you should use TransformersModel instead!",

FutureWarning,

NathanHB · 2024-12-18T15:04:35Z

src/lighteval/models/transformers/transformers_model.py

@@ -124,6 +127,8 @@ class BaseModelConfig:
            model at a quantized precision. Needed for 4-bit and 8-bit precision.
        trust_remote_code (bool): Whether to trust remote code during model
            loading.
+        generation_parameters (GenerationParameters): Range of parameters which will affect the generation.
+        generation_config (GenerationConfig): GenerationConfig object (only passed during manual creation)


I don't understand why we have both generation paramaters and config ?

albertvillanova · 2024-12-18T15:39:46Z

src/lighteval/models/transformers/transformers_model.py

@@ -1297,6 +1348,15 @@ def _loglikelihood_single_token(
        return dataset.get_original_order(res)


+class BaseModel(TransformersModel):
+    def __post_init__(self):
+        super()


Suggested change

super()

super().__post_init__()

albertvillanova · 2024-12-18T15:43:31Z

src/lighteval/main_accelerate.py

@@ -44,7 +44,7 @@ def accelerate(  # noqa C901
    model_args: Annotated[
        str,
        Argument(
-            help="Model arguments in the form key1=value1,key2=value2,... or path to yaml config file (see examples/model_configs/base_model.yaml)"
+            help="Model arguments in the form key1=value1,key2=value2,... or path to yaml config file (see examples/model_configs/transformers_model.yaml)"


Need to rename the file

examples/model_configs/base_model.yaml
to

examples/model_configs/transformers_model.yaml

albertvillanova · 2024-12-18T15:47:31Z

src/lighteval/models/model_input.py

+        if "generation" not in config_dict:
+            return GenerationParameters()
+        return GenerationParameters(
+            early_stopping=config_dict["generation"].get("early_stopping", None),
+            repetition_penalty=config_dict["generation"].get("repetition_penalty", None),
+            frequency_penalty=config_dict["generation"].get("frequency_penalty", None),
+            length_penalty=config_dict["generation"].get("length_penalty", None),
+            presence_penalty=config_dict["generation"].get("presence_penalty", None),
+            max_new_tokens=config_dict["generation"].get("max_new_tokens", None),
+            min_new_tokens=config_dict["generation"].get("min_new_tokens", None),
+            seed=config_dict["generation"].get("seed", None),
+            stop_tokens=config_dict["generation"].get("stop_tokens", None),
+            temperature=config_dict["generation"].get("temperature", None),
+            top_k=config_dict["generation"].get("top_k", None),
+            min_p=config_dict["generation"].get("min_p", None),
+            top_p=config_dict["generation"].get("top_p", None),
+            truncate_prompt=config_dict["generation"].get("truncate_prompt", None),
+        )


Suggested change

if "generation" not in config_dict:

return GenerationParameters()

return GenerationParameters(

early_stopping=config_dict["generation"].get("early_stopping", None),

repetition_penalty=config_dict["generation"].get("repetition_penalty", None),

frequency_penalty=config_dict["generation"].get("frequency_penalty", None),

length_penalty=config_dict["generation"].get("length_penalty", None),

presence_penalty=config_dict["generation"].get("presence_penalty", None),

max_new_tokens=config_dict["generation"].get("max_new_tokens", None),

min_new_tokens=config_dict["generation"].get("min_new_tokens", None),

seed=config_dict["generation"].get("seed", None),

stop_tokens=config_dict["generation"].get("stop_tokens", None),

temperature=config_dict["generation"].get("temperature", None),

top_k=config_dict["generation"].get("top_k", None),

min_p=config_dict["generation"].get("min_p", None),

top_p=config_dict["generation"].get("top_p", None),

truncate_prompt=config_dict["generation"].get("truncate_prompt", None),

)

return GenerationParameters(**config_dict.get("generation", {}))

albertvillanova · 2024-12-18T15:51:29Z

src/lighteval/models/model_input.py

+        args = {
+            "presence_penalty": self.presence_penalty,
+            "frequency_penalty": self.frequency_penalty,
+            "repetition_penalty": self.repetition_penalty,
+            "temperature": self.temperature,
+            "top_p": self.top_p,
+            "top_k": self.top_k,
+            "min_p": self.min_p,
+            "seed": self.seed,
+            "length_penalty": self.length_penalty,
+            "early_stopping": self.early_stopping,
+            "stop": self.stop_tokens,
+            "max_tokens": self.max_new_tokens,
+            "min_tokens": self.min_new_tokens,
+            "truncate_prompt_tokens": self.truncate_prompt,
+        }
+        return {k: v for k, v in args.items() if v is not None}


Suggested change

args = {

"presence_penalty": self.presence_penalty,

"frequency_penalty": self.frequency_penalty,

"repetition_penalty": self.repetition_penalty,

"temperature": self.temperature,

"top_p": self.top_p,

"top_k": self.top_k,

"min_p": self.min_p,

"seed": self.seed,

"length_penalty": self.length_penalty,

"early_stopping": self.early_stopping,

"stop": self.stop_tokens,

"max_tokens": self.max_new_tokens,

"min_tokens": self.min_new_tokens,

"truncate_prompt_tokens": self.truncate_prompt,

}

return {k: v for k, v in args.items() if v is not None}

return {k: v for k, v in asdict(self).items() if v is not None}

albertvillanova · 2024-12-18T15:51:41Z

src/lighteval/models/model_input.py

+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+# SOFTWARE.
+
+from dataclasses import dataclass


Suggested change

from dataclasses import dataclass

from dataclasses import dataclass, asdict

albertvillanova · 2024-12-18T15:58:08Z

src/lighteval/main_endpoint.py

+        with open(model_args, "r") as f:
+            config = yaml.safe_load(f)["model"]
+        generation_parameters = GenerationParameters.from_dict(config)


Maybe worth adding a from_path method?

albertvillanova · 2024-12-18T16:08:01Z

src/lighteval/models/model_input.py

+        }
+        return {k: v for k, v in args.items() if v is not None}
+
+    def to_tgi_inferenceendpoint_dict(self) -> dict:


Maybe renaming to to_tgi_endpoint_dict?

TGI has the word inference included

it's because it's either tgi or inference endpoints. Maybe tgi_ie?

albertvillanova · 2024-12-18T16:16:35Z

src/lighteval/models/model_input.py

+        args = {
+            "max_new_tokens": self.max_new_tokens,
+            "min_new_tokens": self.min_new_tokens,
+            "early_stopping": self.early_stopping or False,


Suggested change

"early_stopping": self.early_stopping or False,

"early_stopping": self.early_stopping,

Why or False? If None, it will not be passed to transformers GenerationConfig and it defaults to False:

https://github.com/huggingface/transformers/blob/052e652d6d53c2b26ffde87e039b723949a53493/src/transformers/generation/configuration_utils.py#L368

clefourrier changed the base branch from main to refacto_model December 9, 2024 14:31

This was referenced Dec 9, 2024

[FT] Add System Prompt field in LightevalTaskConfig that can be used by model clients #410

Open

Allow passing a GenerationConfig for generative evals #16

Open

clefourrier changed the base branch from refacto_model to main December 10, 2024 13:49

clefourrier added 10 commits December 10, 2024 14:55

adding input generation config

1ff5a78

added tgi model

12c6a90

grammar is task dependant, removed from the cofnig

c9657d2

added openai config + moved everything to dict

ac6565a

added generation configs to models

2628571

added generation configs to models

c24bf9b

fix

0aa2e19

fix

e3311bd

added doc

a3f535f

Saved GenerationParameter class in model config classes, then saved i…

286668f

…n the models to use other attributes later

clefourrier force-pushed the clem_homogeneize_generation_params branch from 071d502 to 286668f Compare December 10, 2024 13:55

clefourrier and others added 3 commits December 10, 2024 14:25

changed model args

0b2475a

test

521559f

Merge branch 'main' into clem_homogeneize_generation_params

91363fe

clefourrier and others added 8 commits December 12, 2024 11:30

updated launchers

c088ab6

Merge branch 'main' into clem_homogeneize_generation_params

e1bd34f

rename base_model to transformers_model

3eb7d0f

removed the use of a GenerationConfig object, as it's got lots of par…

a585701

…ams set by default which slow down generations

revert

f9ab29b

fix docs

4833929

fix #16 by also allowing a generationconfig object to be passed progr…

30bed89

…ammatically if needed

Merge branch 'main' into clem_homogeneize_generation_params

431b4f2

clefourrier requested a review from NathanHB December 12, 2024 13:14

albertvillanova reviewed Dec 13, 2024

View reviewed changes

src/lighteval/models/transformers/transformers_model.py Outdated Show resolved Hide resolved

src/lighteval/models/transformers/transformers_model.py Outdated Show resolved Hide resolved

NathanHB reviewed Dec 16, 2024

View reviewed changes

src/lighteval/main_endpoint.py Outdated Show resolved Hide resolved

Apply suggestions from code review

fb4ecdc

Co-authored-by: Nathan Habib <[email protected]> Co-authored-by: Albert Villanova del Moral <[email protected]>

NathanHB reviewed Dec 16, 2024

View reviewed changes

src/lighteval/models/vllm/vllm_model.py Outdated Show resolved Hide resolved

NathanHB reviewed Dec 16, 2024

View reviewed changes

src/lighteval/models/transformers/transformers_model.py Outdated Show resolved Hide resolved

clefourrier and others added 9 commits December 16, 2024 14:43

Update src/lighteval/models/transformers/transformers_model.py

be99c5e

Co-authored-by: Nathan Habib <[email protected]>

Merge branch 'main' into clem_homogeneize_generation_params

e8b9057

removed temperature from default vllm params as it should be passed v…

dece2f9

…ia the generationparams instead

Merge branch 'main' into clem_homogeneize_generation_params

8e3b7e2

Update src/lighteval/models/transformers/transformers_model.py

5c89fe2

Co-authored-by: Albert Villanova del Moral <[email protected]>

logging fix

6a18b81

Merge branch 'main' into clem_homogeneize_generation_params

83cbb10

Merge branch 'main' into clem_homogeneize_generation_params

c6f42ca

added default gen params

90593a9

clefourrier requested a review from NathanHB December 18, 2024 14:34

NathanHB requested changes Dec 18, 2024

View reviewed changes

albertvillanova reviewed Dec 18, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Homogeneize generation params #428

Homogeneize generation params #428

clefourrier commented Dec 9, 2024 •

edited

Loading

HuggingFaceDocBuilderDev commented Dec 9, 2024

clefourrier commented Dec 11, 2024

albertvillanova left a comment

NathanHB left a comment

NathanHB Dec 18, 2024

NathanHB Dec 18, 2024

NathanHB Dec 18, 2024

NathanHB Dec 18, 2024

NathanHB Dec 18, 2024

albertvillanova Dec 18, 2024

albertvillanova Dec 18, 2024

albertvillanova Dec 18, 2024

albertvillanova Dec 18, 2024

albertvillanova Dec 18, 2024

albertvillanova Dec 18, 2024

albertvillanova Dec 18, 2024

clefourrier Dec 26, 2024

albertvillanova Dec 18, 2024

	"Careful, the BaseModel name is deprecated and will be removed, you should use TransformersModel instead!"
	"Careful, the BaseModel name is deprecated and will be removed, you should use TransformersModel instead!",
	FutureWarning,

	from dataclasses import dataclass
	from dataclasses import dataclass, asdict

	"early_stopping": self.early_stopping or False,
	"early_stopping": self.early_stopping,

Homogeneize generation params #428

Are you sure you want to change the base?

Homogeneize generation params #428

Conversation

clefourrier commented Dec 9, 2024 • edited Loading

HuggingFaceDocBuilderDev commented Dec 9, 2024

clefourrier commented Dec 11, 2024

albertvillanova left a comment

Choose a reason for hiding this comment

NathanHB left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clefourrier commented Dec 9, 2024 •

edited

Loading