Fix W&B run name

[qubvel]

What does this PR do?

Currently, the TrainingArguments.run_name argument is silently ignored for W&B if it is the same as TrainingArguments.output_dir.

transformers/src/transformers/integrations/integration_utils.py

Line 766 in d1d94d7

if not (args.run_name is None or args.run_name == args.output_dir):

This behavior was introduced in this PR to avoid runs with the same name in W&B. However,

1. we can still have duplicate run names if their name is not equal to output_dir
2. silent ignoring of an argument may lead to a feeling that the argument is just not working

In my opinion, runs with the same name are normal behavior, they still can be differentiated by hyperparameters and timestamps in UI better than just random names. They also can be renamed manually in UI.

This PR proposes to remove the comparison run_name and output_dir, and always set the run name specified by the user.

Please, let me know what you think.

Env params:

- `transformers` version: 4.41.0.dev0
- Platform: Linux-5.15.0-1056-aws-x86_64-with-glibc2.31
- Python version: 3.10.9
- Huggingface_hub version: 0.22.2
- Safetensors version: 0.4.2
- Accelerate version: 0.29.3

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.

[HuggingFaceDocBuilderDev]

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.

[amyeroberts]

Thanks for this update!

I agree, if I specified run_name I'd expect it to be respected.

cc @muellerzr to confirm it's OK for you

[muellerzr]

How would we feel about as a last option doing output_dir as the run_name, in the unlikely case a user never decides to set it?

Otherwise I'd rather we raise a ValueError and tell the user they must pass in a run_name (potentially breaking)

[qubvel]

I like the idea of doing output_dir as the run_name, but what if someone wants to stay with a random name? Probably it is better to stay with the same behavior as W&B has, if run_name is specified we set it, if not it will be generated randomly.

@parambharat we would much appreciate your thoughts about this change, please let us know if we are missing any of your use cases as you were one of the contributors for this callback 🤗

[muellerzr]

cc @parambharat (right @)

[parambharat]

@qubvel : This check was quite deliberate. The TrainingArguments by default sets the run_name descriptor to the output_dir if the user doesn't set a run_name see here.

That's why the condition here checks if the user deliberately set the run_name i.e. it's not the same as the output_dir Otherwise, it defaults to the output_dir in which case the W&B run would be the output_dir. Therefore, if they, don't set a run_name we default to generating a random name in W&B.

[qubvel]

@parambharat thanks a lot for the clarification, I didn't notice that run_name = output_dir assigned in training arguments!

There might be the following cases with the current code:

1. A user dont set run_name -> they have a random run name in W&B UI, but run_name = output_dir in training arguments
2. A user set run_name = output_dir -> they have unexpected random run name in W&B UI, and run_name = output_dir in training arguments
3. A user set run_name != output_dir -> they have the same name in W&B UI and in training arguments

I suppose we can resolve inconsistency in 1st and 2nd with one of the following ways:

1. Remove assigning run_name = output_dir in training arguments if user did not set run_name (its not working for W&B anyway), and remove the comparison run_name == output_dir in W&B logger. We will get predictable behaviour: if run_name is set we will get it in UI, otherwise, we will get a random name. But this will change behaviour for MLFlowCallabck, now its directly using run_name without comparison with output_dir.
2. Just remove the comparison run_name == output_dir in W&B callback. If run_name is set we will get it in UI, otherwise, we will get output_dir as a run name in W&B UI. No option to get a random name, but I would not expect that there would be too many runs with the same output_dir, so this case is also acceptable. And it will be consistent with MLFlowCallabck.

@muellerzr @parambharat @amyeroberts what do you think?

[amyeroberts]

I like option 2. Option 1 I think is better overall (arguments' behaviour is more consistent), but we want to avoid an unexpected change in behaviour in MLFlowCallback.

[muellerzr]

Thanks!

[parambharat]

Hi @qubvel : One suggestion. Since this is changing the expectation for some W&B users can we add the following warning?

self._wandb.termwarn("Please set `TrainingArguments.run_name` to specify the W&B run name. If unset, `TrainingArguments.output_dir` will be used by default.", repeat=False)

[qubvel]

Ok, added with a bit different wording:

self._wandb.termwarn(
   "The `run_name` is currently set to the same value as `TrainingArguments.output_dir`. If this was "
   "not intended, please specify a different run name by setting the `TrainingArguments.run_name` parameter.",
   repeat=False,
)