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

arg parser doesn't work in a lot of situations #468

Closed
HDCharles opened this issue Apr 24, 2024 · 1 comment · Fixed by #488
Closed

arg parser doesn't work in a lot of situations #468

HDCharles opened this issue Apr 24, 2024 · 1 comment · Fixed by #488
Assignees

Comments

@HDCharles
Copy link
Contributor

see https://github.com/pytorch/torchchat/blob/main/torchchat.py#L98-L104

this code breaks arguments into flag_args and positional_args but makes the assumption that all flag_args will consist of two pieces i.e. -- if the flag_arg doesn't have that form, it causes problems
so something like

... --compile --dtype bfloat16 -> ERROR

since compile only has one part, it throws off the '2 step jump' and stores [--compile, --dtype] as one arg and then sees bfloat16 and thinks its a positional arg. Note if --compile is the last arg it doesn't cause this issue.

... --tasks -> ERROR

the tasks argument has a similar issue unless you use exactly 1 task. If you to --tasks you either

A) if --tasks is not the last argument supplied, you get [--tasks, ] stored as 1 arg and gets parsed as a positional arg
B) if --tasks is the last argument supplied, then no rearrangement happens since its already at the end, but all other positional args get moved after --tasks so when parsed --tasks becomes a list with [, , <positional_arg1>...etc] rather than <positional_arg1> being parsed as a positional arg.

@HDCharles
Copy link
Contributor Author

to solve this we could just detect a list of positional args. to avoid the --tasks B error, we could also insert a dummy positional arg at the end.

malfet added a commit that referenced this issue Apr 25, 2024
And `python3` rather than `python`
- Workaround `--compile` problems by moving it to be last rather than first arg (see #468 )
- Get rid of anti-pattern `run >foo.txt; cat foo.txt`
malfet pushed a commit that referenced this issue Apr 26, 2024
Fixes #468 and #466. Updates named arguments to be registered on subparsers, which allows removal of the arg re-ordering code.
malfet added a commit that referenced this issue Jul 17, 2024
And `python3` rather than `python`
- Workaround `--compile` problems by moving it to be last rather than first arg (see #468 )
- Get rid of anti-pattern `run >foo.txt; cat foo.txt`
malfet pushed a commit that referenced this issue Jul 17, 2024
Fixes #468 and #466. Updates named arguments to be registered on subparsers, which allows removal of the arg re-ordering code.
malfet added a commit that referenced this issue Jul 17, 2024
And `python3` rather than `python`
- Workaround `--compile` problems by moving it to be last rather than first arg (see #468 )
- Get rid of anti-pattern `run >foo.txt; cat foo.txt`
malfet pushed a commit that referenced this issue Jul 17, 2024
Fixes #468 and #466. Updates named arguments to be registered on subparsers, which allows removal of the arg re-ordering code.
malfet added a commit that referenced this issue Jul 17, 2024
And `python3` rather than `python`
- Workaround `--compile` problems by moving it to be last rather than first arg (see #468 )
- Get rid of anti-pattern `run >foo.txt; cat foo.txt`
malfet pushed a commit that referenced this issue Jul 17, 2024
Fixes #468 and #466. Updates named arguments to be registered on subparsers, which allows removal of the arg re-ordering code.
malfet added a commit that referenced this issue Jul 17, 2024
And `python3` rather than `python`
- Workaround `--compile` problems by moving it to be last rather than first arg (see #468 )
- Get rid of anti-pattern `run >foo.txt; cat foo.txt`
malfet pushed a commit that referenced this issue Jul 17, 2024
Fixes #468 and #466. Updates named arguments to be registered on subparsers, which allows removal of the arg re-ordering code.
malfet added a commit that referenced this issue Jul 17, 2024
And `python3` rather than `python`
- Workaround `--compile` problems by moving it to be last rather than first arg (see #468 )
- Get rid of anti-pattern `run >foo.txt; cat foo.txt`
malfet pushed a commit that referenced this issue Jul 17, 2024
Fixes #468 and #466. Updates named arguments to be registered on subparsers, which allows removal of the arg re-ordering code.
malfet added a commit that referenced this issue Jul 17, 2024
And `python3` rather than `python`
- Workaround `--compile` problems by moving it to be last rather than first arg (see #468 )
- Get rid of anti-pattern `run >foo.txt; cat foo.txt`
malfet pushed a commit that referenced this issue Jul 17, 2024
Fixes #468 and #466. Updates named arguments to be registered on subparsers, which allows removal of the arg re-ordering code.
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 a pull request may close this issue.

2 participants