BitsAndBytesConfig Accepting Arbitrary Parameters without Throwing Error

benjaminye commented 8 months ago

System Info

transformers version: 4.38.2
Platform: Linux-6.1.58+-x86_64-with-glibc2.35
Python version: 3.10.12
Huggingface_hub version: 0.20.3
Safetensors version: 0.4.2
Accelerate version: 0.28.0
Accelerate config: not found
PyTorch version (GPU?): 2.2.1+cu121 (True)
Tensorflow version (GPU?): 2.15.0 (True)
Flax version (CPU?/GPU?/TPU?): 0.8.2 (cpu)
Jax version: 0.4.23
JaxLib version: 0.4.23
Using GPU in script?: Yes
Using distributed or parallel set-up in script?: No

Who can help?

@SunMarc @youn

Information

[ ] The official example scripts
[X] My own modified scripts

Tasks

[ ] An officially supported task in the examples folder (such as GLUE/SQuAD, ...)
[X] My own task or dataset (give details below)

Reproduction

Module Imports

from transformers import BitsAndBytesConfig, AutoModel

Define BitsAndBytesConfig

quant_config = BitsAndBytesConfig({"foo":"bar", "foo":"bar"})

# also works with:
# quant_config = BitsAndBytesConfig(**{"foo":"bar", "foo":"bar"})

quant_config
"""
  >>>
  BitsAndBytesConfig {
  "_load_in_4bit": false,
  "_load_in_8bit": {
    "foo": "bar"
  },
  "bnb_4bit_compute_dtype": "float32",
  "bnb_4bit_quant_type": "fp4",
  "bnb_4bit_use_double_quant": false,
  "llm_int8_enable_fp32_cpu_offload": false,
  "llm_int8_has_fp16_weight": false,
  "llm_int8_skip_modules": null,
  "llm_int8_threshold": 6.0,
  "load_in_4bit": false,
  "load_in_8bit": {
    "foo": "bar"
  },
  "quant_method": "bitsandbytes"
}
"""

Load in Model with Quant Config

AutoModel.from_pretrained("TinyLlama/TinyLlama-1.1B-Chat-v1.0", 
                           quantization_config=quant_config, 
                           device_map="auto")

Model successfully loads without any error showing user that an invalid BitsAndBytesConfig was passed in

Expected behavior

It seems like there's no validation for BitsAndBytesConfig, resulting in users able to pass in arbitrary parameters without realizing
Ideally, BitsAndBytesConfig should throw and error when invalid config is encountererd.

amyeroberts commented 8 months ago

cc @younesbelkada

benjaminye commented 8 months ago

The easiest way to prevent this would be having explicit checks when performing post_init() when instantiating BitsAndBytesConfig:

https://github.com/huggingface/transformers/blob/1248f0925234f97da9eee98da2aa22f7b8dbeda1/src/transformers/utils/quantization_config.py#L303-L309

Having checks to see whether load_in_8bit and load_in_4bit positional arguments is an instance of bool would prevent arbitrary types being passed in.
Do we know what the behaviour is when unexpected objects (e.g. dicts) is passed in as load_in_8bit?

I don't think there's a need to check for arguments in case when they are passed in as kwargs, i.e. BitsAndBytesConfig(**{"foo":"bar", "foo":"bar"}). Maybe helpful warning would be nice?

younesbelkada commented 8 months ago

Hi @benjaminye thanks for raising the issue! yes adding a warning makes sense, would you like to open a PR for that? Otherwise happy to do it!

benjaminye commented 8 months ago

Hi @younesbelkada, PR raised @ https://github.com/huggingface/transformers/pull/29761

younesbelkada commented 8 months ago

Thanks so much !

benjaminye commented 8 months ago

Thanks everyone! This was a good first issue to work on 😄

huggingface / transformers