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.

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

Fix for when prompt contains an odd num of apostrophes #4660

Merged
merged 2 commits into from
Nov 15, 2023

Conversation

oelayan7
Copy link
Contributor

@oelayan7 oelayan7 commented Nov 9, 2023

The apostrophes in args.user_args are mixed with the single quotes used to enclose x in the parsing code.

For example,
deepspeed --num_nodes 1 --num_gpus 8 --no_local_rank run_generation.py --model_name_or_path tiiuae/falcon-40b --max_new_tokens 128 --prompt "I'm a student"

instead of (which doesn't work):
deepspeed --num_nodes 1 --num_gpus 8 --no_local_rank run_generation.py --model_name_or_path tiiuae/falcon-40b --max_new_tokens 128 --prompt 'I'm a student'

To resolve this issue, we can make a change to enclose x in double quotes instead of single quotes.

The apostrophes in args.user_args are mixed with the single quotes used to enclose x in the parsing code.

For example,
`deepspeed --num_nodes 1 --num_gpus 8 --no_local_rank run_generation.py --model_name_or_path tiiuae/falcon-40b --max_new_tokens 128 --prompt "I'm a student"`

instead of (which doesn't work):
`deepspeed --num_nodes 1 --num_gpus 8 --no_local_rank run_generation.py --model_name_or_path tiiuae/falcon-40b --max_new_tokens 128 --prompt 'I'm a student'`

To resolve this issue, we can make a change to enclose x in double quotes instead of single quotes.
@tjruwase tjruwase added this pull request to the merge queue Nov 14, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 15, 2023
@loadams loadams enabled auto-merge November 15, 2023 00:38
@loadams loadams added this pull request to the merge queue Nov 15, 2023
@loadams loadams disabled auto-merge November 15, 2023 19:37
Merged via the queue into microsoft:master with commit 00e7dc5 Nov 15, 2023
15 checks passed
@oelayan7 oelayan7 deleted the apostropheFix branch November 16, 2023 11:21
loadams added a commit that referenced this pull request Dec 11, 2023
loadams added a commit that referenced this pull request Dec 11, 2023
mrwyattii added a commit that referenced this pull request Dec 15, 2023
Splitting work from #4769 because we are still debugging transformers
integration issues.

Parsing was broken for user arguments (see #4795). Additionally, parsing
of user arguments is tricky and there are lots of edge cases. For
example: #4660, #4716, #3967. I've attempted to accommodate all of the
possible types of string inputs and added unit tests.
mauryaavinash95 pushed a commit to mauryaavinash95/DeepSpeed that referenced this pull request Feb 17, 2024
The apostrophes in args.user_args are mixed with the single quotes used
to enclose x in the parsing code.

For example,
`deepspeed --num_nodes 1 --num_gpus 8 --no_local_rank run_generation.py
--model_name_or_path tiiuae/falcon-40b --max_new_tokens 128 --prompt
"I'm a student"`

instead of (which doesn't work):
`deepspeed --num_nodes 1 --num_gpus 8 --no_local_rank run_generation.py
--model_name_or_path tiiuae/falcon-40b --max_new_tokens 128 --prompt
'I'm a student'`

To resolve this issue, we can make a change to enclose x in double
quotes instead of single quotes.

Co-authored-by: Logan Adams <[email protected]>
mauryaavinash95 pushed a commit to mauryaavinash95/DeepSpeed that referenced this pull request Feb 17, 2024
Splitting work from microsoft#4769 because we are still debugging transformers
integration issues.

Parsing was broken for user arguments (see microsoft#4795). Additionally, parsing
of user arguments is tricky and there are lots of edge cases. For
example: microsoft#4660, microsoft#4716, microsoft#3967. I've attempted to accommodate all of the
possible types of string inputs and added unit tests.
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.

4 participants