Skip to content

Conversation

@tomaarsen
Copy link
Member

Hello!

What does this PR do?

This PR resolves numerous occurrences of the following:

        raise ValueError(
            "`training_args.block_size` needs to be a multiple of the global train/eval batch size."
            f"Got {training_args.block_size}, {train_batch_size} and {eval_batch_size} respectively instead."
        )

Which results in errors like:

`training_args.block_size` needs to be a multiple of the global train/eval batch size.Got 4, 6 and 12 respectively instead."
                                                                                     ^^

How did I go about it?

I created a simple regex: (["'][^"'\n]*[^ n])(["']\n *f?r?["'][^ ]), which matches full strings that don't end with spaces or n (due to \n), followed by a newline, some spaces, and then another string that also doesn't start with a space. I manually went over all cases where this pattern matched through the codebase, and replaced the pattern with $1 $2 if it was indeed a real mistake.

Before submitting

  • This PR fixes a typo or improves the docs

Who can review?

@ArthurZucker

Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @tomaarsen, thanks, this looks good to me! There were a lot!

@LysandreJik
Copy link
Member

CI error is superfluous, merging!

@LysandreJik LysandreJik merged commit 40ea9ab into huggingface:main Oct 12, 2023
@tomaarsen tomaarsen deleted the chore/missing_spaces branch October 12, 2023 08:30
@tomaarsen
Copy link
Member Author

Awesome! Thanks for getting to this so quickly, these kinds of PRs can become a merge conflict mess if they're left for too long.

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants