switch from `training_args.bin` `training_args.json` #35010

not-lain · 2024-11-29T03:14:06Z

What does this PR do?

switch from training_args.bin to training_args.json and only capture the parameters that the user passed
I'm using the same approach we are using in huggingface_hub's PyTorchModelHubMixin to store as little parameters as possible.
a minimalistic approach to test this is pr

from transformers import TrainingArguments
args = TrainingArguments(output_dir="folder",eval_strategy="no") # or any other paramters
print(args.to_json_string())
# outputs
"""
{
  "output_dir": "folder",
  "eval_strategy": "no",
  "logging_dir": "folder\\runs\\Nov29_02-44-45_iphone-laptop"
}
"""
# logging_dir is a special parameter that is always captured and added to the training_args because we want to ensure consistency

# stores the parameters into a file
args.to_json_file("training_args.json")

# loads an instance using the class directly
args2  = TrainingArguments.from_json_file("training_args.json")

using this approach, we ensure that we only store the parameters that the user-defined manually and not the ones that have default values or the ones inferred from the system (ie cpu, cuda, tpu ... ), leaving some room for flexibility.

in a sense the parameters are mutable, meaning the user can physically alter them.

Fixes #34612

Before submitting

This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
Did you read the contributor guideline,
Pull Request section?
Was this discussed/approved via a Github issue or the forum? Please add a link
to it if that's the case.
Did you make sure to update the documentation with your changes? Here are the
documentation guidelines, and
here are tips on formatting docstrings.
Did you write any new necessary tests?

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@muellerzr @SunMarc

src/transformers/training_args.py

not-lain · 2024-11-29T17:21:57Z

cc @muellerzr & @SunMarc for review.
failed test is unrelated

not-lain · 2024-12-09T15:40:56Z

friendly tagging @muellerzr & @SunMarc
failed tests are unrelated

muellerzr

Very nice! I'm a fan of this change. However, we should be careful about old training resumptions from older versions and likely should do a deprecation until 5.0.0 (warning if a .bin was found but we do the shift to .json now)

How does that sound?

HuggingFaceDocBuilderDev · 2024-12-11T03:06:31Z

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.

not-lain · 2024-12-12T01:44:33Z

[don't merge yet I need to fix something fundamental that was not captured by the tests]

not-lain · 2024-12-12T02:19:58Z

cc @muellerzr for feedback
I implemented a rollback, now default is traing_args.bin and people can switch to training_args.json by setting the TRAINER_SAFE_SERIALIZE environmental variable.

originally there was never a mechanism inside of the trainer API to load any training_args and people used to load the training arguments manually, I added something useful which is TrainingArguments.from_json_file(file_name )
but I think I will integrate this loading mechanism with the trainer api in another pr (optional new feature)

not-lain · 2024-12-16T13:09:54Z

friendly tagging @muellerzr here for a review.

not-lain · 2024-12-23T17:13:21Z

cc @muellerzr for review

SunMarc

Thanks for the PR ! Left a few comments !

src/transformers/trainer.py

SunMarc · 2024-12-24T14:51:26Z

src/transformers/training_args.py

-            if is_accelerate_available() and isinstance(v, AcceleratorConfig):
-                d[k] = v.to_dict()
+            d[k] = serialize_parameter(k, v)
        self._dict_torch_dtype_to_str(d)


We can remove this and the function since you took care of that in serialize_parameter. Can you add a comment in serialize_parameter to explain what is happening to torch_dtype ?

I hope I did not misunderstand you, let me know if this has been resolved after the new changes

Looks fine thanks ! SInce we are not using _dict_torch_dtype_to_str method, you can also remove it in this PR

tests/trainer/test_trainer.py

Co-authored-by: Marc Sun <[email protected]>

…/not-lain/transformers into switch-training_args-file-format

not-lain · 2025-01-21T19:26:13Z

the test_missing_stateful_callback is now failing, any recommendations on how to approach this?

…/not-lain/transformers into switch-training_args-file-format

not-lain · 2025-02-07T03:06:31Z

cc @muellerzr , failing tests are unrelated

not-lain · 2025-02-12T14:31:57Z

friendly tagging @muellerzr for review

not-lain · 2025-02-19T03:56:17Z

cc @muellerzr for review

not-lain · 2025-02-25T14:47:40Z

friendly tagging @muellerzr here for review

not-lain · 2025-03-04T09:24:00Z

cc @muellerzr to see if you can review this

muellerzr

Thanks a bunch! fstring nit then passing off to @ArthurZucker :)

(Merged myself though so we can get this in without more delay!)

src/transformers/trainer.py

not-lain · 2025-03-10T16:21:21Z

cc @ArthurZucker for review 🤗

ArthurZucker

I have no idea where this stands, happy to merge

not-lain · 2025-04-08T18:10:18Z

this pr is related to https://github.com/huggingface/transformers/pull/35010/files#diff-bfceaff300c851b8e24fc50dc6638482abaec8f7d2a718e877c3828c166bcf79L223 where it said :

TODO: TrainingArguments users rely on it being fully mutable. In the future see if we can narrow this to a few keys: #25903

in short this pr :

switch from bin -> json (better security)
training args become fully mutable
dynamic parameter serialization (storing only key parameters) (see switch from training_args.bin training_args.json #35010 (comment) for more details )

do not hesitate if you have any questions or follow-ups 🤗

SunMarc · 2025-04-09T12:27:39Z

Can you fix the conflicts ? Then, we will finally be able to merge it ;)

not-lain · 2025-04-09T19:06:16Z

done (≧∇≦)ﾉ

SunMarc · 2025-04-10T12:33:57Z

src/transformers/training_args.py

+    def __new__(self, *args, **kwargs):
+        # catch and save only the parameters that the user passed
+        self.__training_args_params__ = {}
+        param_names = list(self.__dataclass_fields__.keys())
+
+        for i in range(len(args)):
+            self.__training_args_params__[param_names[i]] = serialize_parameter(param_names[i], args[i])
+
+        for k, v in kwargs.items():
+            self.__training_args_params__[k] = serialize_parameter(k, v)
+
+        return super().__new__(self)
+


just a caveat of only storing training params that were passed, if we change the default of any argument in the future, it will impact the training of the model if the user try to resume the training no ? it is a pretty small edge case but still nice to be aware of it.
Also, does this happen with the bin file ?

I can't deny the fact that when we change the default params the default training arguments will change, but still what if we remove some of the default parameters, the approach introduced in this pr is still more flexible (note that the transformers version and some of these parameters are already part of the readme file so that the user can always have a fallback if they want to )

another good example of how this shows more prospects is that if we change the training device, if the second server supports more default cores then that will be default parameter over there (checkout tpu_num_cores for reference)

In full transparency, in bin, everything is static, and i haven't seen anyone report any problems with it.

Thanks for the details, as this might impact heavily 3rd party library relying on trainer, I'll ask them if this is not too breaking ! cc @qgallouedec @winglian @danielhanchen

Thanks @SunMarc, no change required on TRL side 👌

not-lain and others added 5 commits November 29, 2024 01:45

capture init parameters in training_args

a6484c0

update relevant attributes

780d3e7

attribute calling for training args

66c655b

add class attribute to load the training_args from a local file

6d3a186

Merge branch 'main' into switch-training_args-file-format

2694533

not-lain commented Nov 29, 2024

View reviewed changes

src/transformers/training_args.py Outdated Show resolved Hide resolved

Update src/transformers/training_args.py

5acc885

Merge branch 'main' into switch-training_args-file-format

110e303

muellerzr reviewed Dec 11, 2024

View reviewed changes

not-lain and others added 7 commits December 11, 2024 22:06

Merge branch 'huggingface:main' into switch-training_args-file-format

9470d77

ensure backward compatibility

79d2a3a

fix version parsing

c7567f7

fix deprecated filename

68089ed

update test

2a6ee0d

format with ruff

8228d0f

fix trainer logger tests

ba373dc

serialize all parameters

10b05dd

SunMarc reviewed Dec 24, 2024

View reviewed changes

not-lain and others added 5 commits December 24, 2024 16:55

Update src/transformers/trainer.py

7633c80

Co-authored-by: Marc Sun <[email protected]>

Refactor serialization logic in Trainer to use binary serialization flag

e3c9e57

Merge branch 'main' into switch-training_args-file-format

0d4366b

format with ruff

689348d

Merge branch 'switch-training_args-file-format' of https://github.com…

56bdbca

…/not-lain/transformers into switch-training_args-file-format

not-lain and others added 4 commits February 7, 2025 03:17

update test

63d7259

Merge branch 'main' into switch-training_args-file-format

daa2381

thanks claude

b384048

Merge branch 'switch-training_args-file-format' of https://github.com…

f5d73ee

…/not-lain/transformers into switch-training_args-file-format

Merge branch 'main' into switch-training_args-file-format

94af0d9

SunMarc requested a review from muellerzr February 7, 2025 13:45

Merge branch 'main' into switch-training_args-file-format

249f092

muellerzr approved these changes Mar 5, 2025

View reviewed changes

src/transformers/trainer.py Outdated Show resolved Hide resolved

Update src/transformers/trainer.py

5a45457

muellerzr requested a review from ArthurZucker March 5, 2025 08:20

not-lain commented Mar 5, 2025

View reviewed changes

src/transformers/trainer.py Outdated Show resolved Hide resolved

src/transformers/trainer.py Outdated Show resolved Hide resolved

src/transformers/trainer.py Outdated Show resolved Hide resolved

not-lain added 4 commits March 5, 2025 10:19

Update src/transformers/trainer.py

c36900d

Update src/transformers/trainer.py

8106816

Update src/transformers/trainer.py

6b7755a

Merge branch 'main' into switch-training_args-file-format

9a8c9d4

ArthurZucker approved these changes Apr 8, 2025

View reviewed changes

not-lain force-pushed the switch-training_args-file-format branch from c468866 to 9a8c9d4 Compare April 9, 2025 18:15

not-lain and others added 2 commits April 9, 2025 19:21

resolve merge conflict

8f578db

Merge branch 'main' into switch-training_args-file-format

62c6abc

SunMarc reviewed Apr 10, 2025

View reviewed changes

switch from training_args.bin training_args.json #35010

Are you sure you want to change the base?

switch from training_args.bin training_args.json #35010

Uh oh!

Conversation

not-lain commented Nov 29, 2024

What does this PR do?

Before submitting

Who can review?

Uh oh!

Uh oh!

not-lain commented Nov 29, 2024

Uh oh!

not-lain commented Dec 9, 2024

Uh oh!

muellerzr left a comment

Choose a reason for hiding this comment

Uh oh!

HuggingFaceDocBuilderDev commented Dec 11, 2024

Uh oh!

not-lain commented Dec 12, 2024

Uh oh!

not-lain commented Dec 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

not-lain commented Dec 16, 2024

Uh oh!

not-lain commented Dec 23, 2024

Uh oh!

SunMarc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

SunMarc Dec 24, 2024

Choose a reason for hiding this comment

Uh oh!

not-lain Dec 27, 2024

Choose a reason for hiding this comment

Uh oh!

SunMarc Dec 30, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

not-lain commented Jan 21, 2025

Uh oh!

not-lain commented Feb 7, 2025

Uh oh!

not-lain commented Feb 12, 2025

Uh oh!

not-lain commented Feb 19, 2025

Uh oh!

not-lain commented Feb 25, 2025

Uh oh!

not-lain commented Mar 4, 2025

Uh oh!

muellerzr left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

not-lain commented Mar 10, 2025

Uh oh!

ArthurZucker left a comment

Choose a reason for hiding this comment

Uh oh!

not-lain commented Apr 8, 2025

Uh oh!

SunMarc commented Apr 9, 2025

Uh oh!

not-lain commented Apr 9, 2025

Uh oh!

SunMarc Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

not-lain Apr 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

switch from `training_args.bin` `training_args.json` #35010

switch from `training_args.bin` `training_args.json` #35010

not-lain commented Dec 12, 2024 •

edited

Loading

muellerzr left a comment •

edited

Loading

not-lain Apr 14, 2025 •

edited

Loading