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

Modify the args_default usage #250

Merged
merged 1 commit into from
Sep 21, 2023
Merged

Modify the args_default usage #250

merged 1 commit into from
Sep 21, 2023

Conversation

yuanwu2017
Copy link

While args_defaults is compatible with the original usage, the value of argments can also be set through function call usage.

While args_defaults is compatible with the original usage,
the value of argments can also be set through function call usage.

Signed-off-by: yuanwu <[email protected]>
@conglongli conglongli self-assigned this Sep 18, 2023
@conglongli
Copy link

@yuanwu2017 your description is unclear. If your PR fixes an existing issue, please describe what issue does it fix with an example. If your PR is a new feature, please give an example of when it is helpful.

@yuanwu2017
Copy link
Author

We are integrating the Megatron-deepspeed into Ray. We need initialize_megatron to initialize megatron by passing in function arguments instead of arguments from the startup command.
For example:

args = {
  "num_layers": 32,
            "hidden_size": 4096,
            "ffn_hidden_size": 11008,
            "num_attention_heads": 32,
            "max_position_embeddings":4096,
            "num_key_value_heads": 32
              .......
}
initialize_megatron(ignore_unknown_args=True, args_defaults=args)

@conglongli conglongli merged commit 3095a51 into microsoft:main Sep 21, 2023
1 check passed
@JiafeiSun
Copy link

with the patch, I run pretrain_llama2_distributed.sh which set --tokenizer-type GPTSentencePieceTokenizer, but in pretrain_gpt.py args_defaults={'tokenizer_type': 'GPT2BPETokenizer'}, as result, tokenizer-type=GPT2BPETokenizer, but for llama2, tokenizer-type should be GPTSentencePieceTokenizer, how to solve it? ths

yuanwu2017 added a commit to yuanwu2017/Megatron-DeepSpeed that referenced this pull request Jan 5, 2024
yuanwu2017 added a commit to yuanwu2017/Megatron-DeepSpeed that referenced this pull request Jan 5, 2024
@yuanwu2017
Copy link
Author

Sorry for the introduction of bug and your inconvenience. I reverted the patch and made a new one.

@JiafeiSun
Copy link

Sorry for the introduction of bug and your inconvenience. I reverted the patch and made a new one.

thx

conglongli pushed a commit that referenced this pull request Jan 5, 2024
* Revert "Modify the args_default usage (#250)"

This reverts commit 3095a51.

* Add the external arguments

Add the external_arguments for passing the arguments from function call.

Signed-off-by: yuanwu <[email protected]>

---------

Signed-off-by: yuanwu <[email protected]>
zdaiot pushed a commit to zdaiot/Megatron-DeepSpeed that referenced this pull request Jan 20, 2024
* Revert "Modify the args_default usage (microsoft#250)"

This reverts commit 3095a51.

* Add the external arguments

Add the external_arguments for passing the arguments from function call.

Signed-off-by: yuanwu <[email protected]>

---------

Signed-off-by: yuanwu <[email protected]>
zdaiot pushed a commit to zdaiot/Megatron-DeepSpeed that referenced this pull request Jan 25, 2024
* Revert "Modify the args_default usage (microsoft#250)"

This reverts commit 3095a51.

* Add the external arguments

Add the external_arguments for passing the arguments from function call.

Signed-off-by: yuanwu <[email protected]>

---------

Signed-off-by: yuanwu <[email protected]>
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