-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Flops profiler support einops.einsum #6755
base: master
Are you sure you want to change the base?
Flops profiler support einops.einsum #6755
Conversation
lvhoaa
commented
Nov 17, 2024
- Added support for FlopsProfiler to include einops.einsum operation
- Added _patch_miscellaneous_operations() and _reload_miscellaneous_operations() to include this operation and potentially include other miscellaneous operations in the future
- Added _einops_einsum_flops_compute() that mimic already-existed _einsum_flops_compute()
@microsoft-github-policy-service agree |
@@ -16,6 +16,8 @@ | |||
from deepspeed.moe.layer import MoE | |||
from deepspeed.utils.timer import FORWARD_GLOBAL_TIMER, BACKWARD_GLOBAL_TIMER, STEP_GLOBAL_TIMER | |||
from deepspeed.utils.torch import required_torch_version | |||
import einops |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @lvhoaa - could you run the pre-commit formatter on the branch to ensure the formatting check passes?
Also einops
is not included in the requirements file, so it will need to be added there otherwise the tests will fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@loadams Thanks for the feedback! I have pushed a fix!
bin/deepspeed
Outdated
ds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lvhoaa - I can't tell why these files were changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@loadams I really didn't intentionally change them. It's probably the pre-commit. Have fixed.
requirements/requirements.txt
Outdated
@@ -1,3 +1,4 @@ | |||
einops |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this would probably be better suited to the requirements-dev file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
…com/lvhoaa/DeepSpeed into FlopsProfiler-support-einops.einsum