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

Fix attribute error when loading FusedAdamBuilder() #3527

Merged
merged 5 commits into from May 16, 2023

Conversation

rraminen
Copy link
Contributor

@rraminen rraminen commented May 12, 2023

This PR is to avoid the below error when loading FusedAdamBuilder() extension.

    deepspeed.ops.op_builder.FusedAdamBuilder().load()
  File "/root/DeepSpeed/op_builder/builder.py", line 453, in load
    return self.jit_load(verbose)
  File "/root/DeepSpeed/op_builder/builder.py", line 492, in jit_load
    if not self.build_for_cpu and self.enable_bf16:
AttributeError: 'FusedAdamBuilder' object has no attribute 'enable_bf16'

cc: @jithunnair-amd

Copy link
Contributor

@jithunnair-amd jithunnair-amd left a comment

Choose a reason for hiding this comment

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

@rraminen thanks for the fix. It looks like this issue should occur for both CUDA and ROCm?

@rraminen
Copy link
Contributor Author

@rraminen thanks for the fix. It looks like this issue should occur for both CUDA and ROCm?

I have not tested it on CUDA, but it looks like it would occur on CUDA too.

@@ -107,6 +107,7 @@ def __init__(self, name):
self.name = name
self.jit_mode = False
self.build_for_cpu = False
self.enable_bf16 = False
Copy link
Contributor

Choose a reason for hiding this comment

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

@jeffra This is being done in the constructor for OpBuilder class, even though the enable_bf16 attributed is really only checked for CUDAOpBuilder:

if not self.build_for_cpu and self.enable_bf16:

The issue is that the initialization of that attribute happens inside https://github.com/microsoft/DeepSpeed/blob/82991df05419828d65cbb658efc01b50c135ea00/op_builder/builder.py#LL517C9-L517C32, which is only called for CUDA.

Would you prefer to initialize this in OpBuilder or CUDAOpBuilder?

Copy link
Contributor

Choose a reason for hiding this comment

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

@tjruwase tjruwase merged commit 2fc5684 into microsoft:master May 16, 2023
19 checks passed
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.

None yet

4 participants