Uh oh!
There was an error while loading. Please reload this page.
FP8 Support for MCore MoE - #648
Conversation
9163446 to
50d180cCompare9b520c6 to
8df68fcCompare42f28d3 to
8e03976Compareptrendx
commented
Mar 7, 2024
I don't like the fact that the layers need to know that they are experts. Can't it be abstracted in some way using the options that we already have or add options that are more generic? |
Victarry
commented
Mar 8, 2024
I see. Good advice. I remove the |
Victarry
commented
Mar 12, 2024
Hi @ptrendx, could you please continue the review and share your comments. To make sure this feature can be included in MCore v0.6, I think it's better to merge this MR this week. |
ptrendx
commented
Mar 13, 2024
So, to be honest, I don't quite understand why we need that communication flag at all. MCore should be able to just call te.Linear without setting row or column parallelism to the same effect, no? And then we would not need any special flag on the TE side? Also, you added this rng tracker name option, but did not document it. Handling of the zero token case I think is fine. |
Victarry
commented
Mar 14, 2024
Added documentation. Thanks. |
ptrendx
commented
Mar 18, 2024
/te-ci pytorch |
Victarry
commented
Mar 25, 2024
Hi @ptrendx, I have added the new unittest for Linear layer with empty input. Since I didn't find an appropriate file to place the new testcase, I created a new test file named |
ptrendx
commented
Mar 25, 2024
I would put it in test_sanity.py. It looks good, but please also add some check in it - like checking that the batch size of the output of the linear layer is the same as the input (so if 0 gets passed as input, 0 is also provided as the output). |
Victarry
commented
Mar 26, 2024
Done! Thanks for you advice. 👍🏻 |
Victarry
commented
Apr 7, 2024
Hi, @ptrendx, can this MR be merged now? |
ptrendx
commented
Apr 7, 2024
/te-ci pytorch |
Victarry
commented
Apr 7, 2024
Hi, @ptrendx, I just fixed the UT error in CI, could you please trigger the ci again? |
ptrendx
commented
Apr 7, 2024
/te-ci pytorch |
Victarry
commented
Apr 8, 2024
Hi, @ptrendx, the CI pipeline is passed, could you please merge this MR? |
ptrendx
commented
Apr 8, 2024
Hi @Victarry, we are trying to minimize the changes going into 1.6 release so will merge that PR after 1.6 branch is created. |
Signed-off-by: Dennis Liu <denliu@nvidia.com>
Signed-off-by: Dennis Liu <denliu@nvidia.com>
Signed-off-by: Dennis Liu <denliu@nvidia.com>
ptrendx
commented
Apr 16, 2024
Hi @Victarry, now that the 1.6 branch is created, could you resolve conflicts in your PR? Then we will be able to merge it. |
Victarry
commented
Apr 17, 2024
Hi, @ptrendx, I just resolved the conflicts, please merge this PR. Thanks! |
ptrendx
commented
Apr 17, 2024
/te-ci pytorch |
Victarry
commented
Apr 24, 2024
Hi @ptrendx, I guess the CI failure is due to other code change in main branch. Could you please trigger the pytorch CI again? |
ptrendx
commented
Apr 25, 2024
/te-ci pytorch |
viclzhu
commented
Apr 25, 2024
Hi @Victarry, I'm wondering when the mcore related changes will be available on the public mcore repository. Or if it's already available, could you point me to the relevant changes or PR? Thanks! |
Victarry
commented
Apr 29, 2024
Hi, @viclzhu, the mcore related change is planed to be published before the end of May. |
Victarry
commented
Apr 29, 2024
Hi @ptrendx, I found that the UT only failed on L40, but I'm not sure why does this happen. Do you have any insights? |
ptrendx
commented
Apr 29, 2024
Hi Victarry - I checked and this failure is unrelated to this PR, so I believe it is safe to merge. |
* Add support for MoE with FP8. Signed-off-by: Dennis Liu <denliu@nvidia.com> * Fix unittest. Signed-off-by: Dennis Liu <denliu@nvidia.com> * Fix error in linear backward. Signed-off-by: Dennis Liu <denliu@nvidia.com> --------- Signed-off-by: Dennis Liu <denliu@nvidia.com> Co-authored-by: Przemyslaw Tredak <ptredak@nvidia.com>
* Add support for MoE with FP8. Signed-off-by: Dennis Liu <denliu@nvidia.com> * Fix unittest. Signed-off-by: Dennis Liu <denliu@nvidia.com> * Fix error in linear backward. Signed-off-by: Dennis Liu <denliu@nvidia.com> --------- Signed-off-by: Dennis Liu <denliu@nvidia.com> Co-authored-by: Przemyslaw Tredak <ptredak@nvidia.com> Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>
| size_t M = static_cast<size_t>(input.size(0)); | ||
| size_t N = static_cast<size_t>(input.size(1)); | ||
| if (M == 0 || N == 0) | ||
| return input; |
There was a problem hiding this comment.
@Victarry This will cause shape mismatch error between wgrad and weight when gradient accumulation fusion is disabled.
Add FP8 support for MoE in MCore.
Related MR in MCore:
https://gitlab-master.nvidia.com/ADLR/megatron-lm/-/merge_requests/1089
Implementation details:
rng_tracker_namefor initialize with EP