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

Improve run_llama_train.sh args and add local-ranks-filter #51

Merged
merged 1 commit into from
Feb 10, 2024

Conversation

wconstab
Copy link
Contributor

@wconstab wconstab commented Feb 9, 2024

allows convenient switching of args w/o editing .sh file
e.g.
LOG_RANK=1,2 ./run_llama_train.sh
NGPU=2 SP=2 ./run_llama_train.sh

@wconstab wconstab requested a review from wanchaol February 9, 2024 23:38
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Feb 9, 2024
Comment on lines +13 to +15
PP=${PP:-"1"}
SP=${SP:-"1"}
DP=${DP:-"-1"}
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder where these are picked up. Shall we also add to the torchrun cmd? e.g. --pp_degree=${PP}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, yea, i forgot to copy that part over. will fix.

Copy link
Contributor

@tianyu-l tianyu-l left a comment

Choose a reason for hiding this comment

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

Had an inline comment; o/w looks all good. Thanks for working on this!

DP=${DP:-"-1"}

# by default log just rank 0 output,
LOG_RANK=${LOG_RANK:-0}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think one issue for my solution is that you can't easily disable 'log_rank' unless you comment out the --local-ranks-filter option

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gonna merge it this way, its probably easier to have rank0 by default and comment out than remember how to add the filter string in, but we can probably improve this.

@wconstab wconstab merged commit da50d34 into main Feb 10, 2024
3 checks passed
@wconstab wconstab deleted the whc/logrank branch February 10, 2024 00:51
lessw2020 pushed a commit that referenced this pull request Apr 18, 2024
allows convenient switching of args w/o editing .sh file
e.g.
`LOG_RANK=1,2 ./run_llama_train.sh`
`NGPU=2 SP=2 ./run_llama_train.sh`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants