Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make output_dir optional in TrainingArguments #27866

Open
ChanderG opened this issue Dec 6, 2023 · 7 comments · May be fixed by #35735
Open

Make output_dir optional in TrainingArguments #27866

ChanderG opened this issue Dec 6, 2023 · 7 comments · May be fixed by #35735
Labels
Feature request Request for a new feature Good Second Issue Issues that are more difficult to do than "Good First" issues - give it a try if you want!

Comments

@ChanderG
Copy link
Contributor

ChanderG commented Dec 6, 2023

Feature request

Currently, there is only 1 required param in creating a TrainingArguments object - output_dir. HFTrainer manually creates an object with a default value "tmp_trainer" if no Args object is passed to it.

Instead, we should make even this one param optional in the TrainingArguments class (and use a default inside the class implementation).

Motivation

This is useful when creating and passing TrainingArguments in other runners - for eg, trl/SFTTrainer. I would like sensible defaults for all params, so that I only specify the particular arguments I am interested in.

Your contribution

I can open a PR, if this is of interest.
@ArthurZucker
Copy link
Collaborator

cc @muellerzr sounds good to me wdyt?

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 6, 2024

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.

@ArthurZucker ArthurZucker added the Feature request Request for a new feature label Jan 8, 2024
@artshcherbina
Copy link

I've found a workaround using inheritance:

@dataclass
class MyTrainingArguments(TrainingArguments):
    output_dir: str = field(
        default="models",
    )

@huynhducloi00
Copy link

not everyone want to save the model

@ArthurZucker ArthurZucker added the Good Second Issue Issues that are more difficult to do than "Good First" issues - give it a try if you want! label Jan 16, 2025
@ArthurZucker
Copy link
Collaborator

Let's leave it to the community!

@sambhavnoobcoder
Copy link

i'll take this up .

@sambhavnoobcoder sambhavnoobcoder linked a pull request Jan 16, 2025 that will close this issue
Open
@sambhavnoobcoder
Copy link

raised a PR #35735 to fix this issue . maintainers kindly give this a look .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Feature request Request for a new feature Good Second Issue Issues that are more difficult to do than "Good First" issues - give it a try if you want!
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Make output_dir Optional in TrainingArguments #27866 sambhavnoobcoder/transformers
5 participants
@artshcherbina @ChanderG @ArthurZucker @huynhducloi00 @sambhavnoobcoder