alignment-handbook
alignment-handbook copied to clipboard
Fix BitsAndBytes JSON Serializable
This PR is related to #189
Current transformers tries to convert every config objects into Dictionary(link), but it does not resolve in nested config case (BitsAndBytesConfig) with the following error. This is because BitsAndBytesConfig is stored under quantization_config of the training args:
TypeError: Object of type BitsAndBytesConfig is not JSON serializable
caused by [this(https://github.com/huggingface/transformers/blob/e0d82534cc95b582ab072c1bbc060852ba7f9d51/src/transformers/training_args.py#L2446)].
The easiest solution for this is to call to_dict() method after get_quantization_config().
thanks @deep-diver do you think it makes more sense to change the get_quantization_config method to return a dict? Then one would just need to update the tests for this function and it could be a cleaner way?
@kashif and @alvarobartt, Thanks for the comments.
scripts are not provided as a part of alignment-handbook package while other parts are. By changing the internal code base of the package could break other applications' behaviour. So, I made a small change inside the run_sft.py.
That is how I perceived about this. But, I am OK with you guys' proposes too.
as we tag the lib. with versions perhaps with this change we can tag it to a new version... so lets update the API to return dicts
@kashif
removed to_dict() call from the scripts, and add to_dict() in get_quantization_config()`
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.
Hi here @deep-diver thanks for this PR! Indeed there's a broken unit test:
tests/test_model_utils.py::GetQuantizationConfigTest::test_8bit - AttributeError: 'dict' object has no attribute 'load_in_8bit'
If you could fix that before merging that would be great! I can create a PR to fix the rest of the unit tests as one's already reported at https://github.com/huggingface/alignment-handbook/issues/166.
Ah the test is failing because it previously assumed quantization_config was an object. Can you fix the test please and then we merge?
Tested out and made sure everything works ok. @lewtun @alvarobartt
Thanks for iterating!