Add getter and setter methods for compile_backend across accelerators.

[vshekhawat-hlab]

Add getter and setter methods for compile_backend across accelerators, which provide a mechanism to retrieve the compile backend. These APIs handle user-defined backend selection and raise a ValueError with informative error messages for unsupported backends.

[vshekhawat-hlab]

Hi @loadams ,

Added support for retrieving the preferred compile backend from the accelerator. However, I'm unsure about the preferred backend for each accelerator. Therefore, I've set the default to 'inductor'. Could you please review this?

[tjruwase]

@vshekhawat-hlab, thanks for adding this API. I think it is a great start to addressing generalization issue raised by @delock. However, there is a bit more work required to make this consistent with

DeepSpeed/deepspeed/runtime/compiler.py

Line 56 in 3dd3d51

class CompileConfig(DeepSpeedConfigModel):

.
Also, I think there are few questions that need resolving. I will post them below as separate comment to reduce clutter.

[tjruwase]

Since users can specify compile_backend (string or Callable) through either ds_config or set_backend(), which value takes precedence between user specification and preferred/default backend in accelerator class?

I think user specification should have higher precedence.

[tjruwase]

Assuming user specification has higher precedence, how can users select the accelerator preferred backend without having to read the code? I see two options:

1. Users can implicitly select the accelerator preferred backend by leaving backend undefined in ds_config, and by not using set_backend() API, or
2. Alternatively, users can explicitly use a special backend name, e.g., "accelerator".

[tjruwase]

What is the expected behavior if user specifies backend that is not supported by accelerator? I see three options:

1. Fail immediately with meaningful error message.
2. Disable compilation with warning message.
3. Switch to accelerator preferred backend with warning message.

[tjruwase]

@delock, @tohtana, @umchand, @vshekhawat-hlab, @loadams, I will appreciate your thoughts on the above questions. Thanks!

[vshekhawat-hlab]

Thanks @tjruwase for review.

My views on above:
Query1) Yes, i think user specification should get higher precedence.
Query3) I think option 1, as the user is expected to run with the user-specified backend, that is the reason user passed the backend. However option3 is also good option.

Updated get_compile_backend, let me know your views on this.

def get_compile_backend(self, backend=None):
    supported_backends = torch._dynamo.list_backends()
    if backend is None:
        return preferred_backend
    elif backend in supported_backends:
        return backend
    else:
        raise ValueError(f"{backend} not supported by {self.device_name()}. Supported Backends are {supported_backends }") 

`

[loadams]

Thanks @tjruwase for review.

My views on above: Query1) Yes, i think user specification should get higher precedence. Query3) I think option 1, as the user is expected to run with the user-specified backend, that is the reason user passed the backend. However option3 is also good option.

Updated get_compile_backend, let me know your views on this.

def get_compile_backend(self, backend=None):
    supported_backends = torch._dynamo.list_backends()
    if backend is None:
        return preferred_backend
    elif backend in supported_backends:
        return backend
    else:
        raise ValueError(f"{backend} not supported by {self.device_name()}. Supported Backends are {supported_backends }") 

`

I agree on this, I think the best option if the user specifies a backend that is not supported is to fail immediately with a meaningful error message.

And I think having it be implicit except if they override in ds_config or via set_backend() would then take precedence.

[delock]

Agree that if user explicitly specifiy backend then should fail immedately. Also leaving backend undefined as indication for default value is a sound behavior.

[vshekhawat-hlab]

@tjruwase , @loadams , @delock ,

Can you please review the change. Is the current change is okay?

[loadams]

@tjruwase , @loadams , @delock ,

Can you please review the change. Is the current change is okay?

Thanks @vshekhawat-hlab, we will work on reviewing this.

[tjruwase]

@vshekhawat-hlab, apologies for the delay. Based on all the feedback, the main change needed in my mind is to split into a get_compile_backend() and set_compile_backend(), so that the get_ API can be side effect-free.

1. get_compile_backend() should return the current backend of the accelerator.
2. Each accelerator can initialize the current backend as appropriate, probably to the default/preferred value.
3. set_compile_backend() allows users to modify the backend. It should perform the necessary sanity checks, including those performed by current get_ API

@vshekhawat-hlab, @tohtana, @umchand, @delock, @loadams, I will appreciate your thoughts on the above.

[loadams]

@vshekhawat-hlab, apologies for the delay. Based on all the feedback, the main change needed in my mind is to split into a get_compile_backend() and set_compile_backend(), so that the get_ API can be side effect-free.

1. get_compile_backend() should return the current backend of the accelerator.
2. Each accelerator can initialize the current backend as appropriate, probably to the default/preferred value.
3. set_compile_backend() allows users to modify the backend. It should perform the necessary sanity checks, including those performed by current get_ API

@vshekhawat-hlab, @tohtana, @umchand, @delock, @loadams, I will appreciate your thoughts on the above.

I like that approach, I think it makes the most sense to match existing APIs and provide default behavior and a normal way to override it.

[vshekhawat-hlab]

Hi @tjruwase,

Agree with comment.

Couple of questions:

• In set API, user has to pass backend, None args can't be allowed here. Right?
• Do you see any more checks that can be added in set API?

Code snippet with get and set API.

    def get_compile_backend(self):
        return self.compile_backend

    def set_compile_backend(self, backend):
        supported_backends = torch._dynamo.list_backends()
        if backend in supported_backends:
            self.compile_backend = backend
        else:
            raise ValueError(
                f"{backend} not supported by {self.device_name()}. Supported Backends are {supported_backends}")

[tjruwase]

• In set API, user has to pass backend, None args can't be allowed here. Right?

Yes, it makes sense to reject None backend.

[tjruwase]

@vshekhawat-hlab, the proposed snippet looks good to me. I suspect that individual accelerators might later apply optimizations such as caching the supported backends list in the constructor. But your current proposal is a simple and clean start. Thanks for helping.

[vshekhawat-hlab]

@vshekhawat-hlab, the proposed snippet looks good to me. I suspect that individual accelerators might later apply optimizations such as caching the supported backends list in the constructor. But your current proposal is a simple and clean start. Thanks for helping.

Updated the PR as discussed. Please review.