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 DirectML_ESRGAN's default adapter selection. #667

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wonchung-microsoft
Copy link

Modify DirectML_ESRGAN sample to by default try NPU first then retry GPU. If adapter specified through param, stay with the specified param.

@wonchung-microsoft
Copy link
Author

@microsoft-github-policy-service agree company="Microsoft"

Copy link

@ashrit-ms ashrit-ms left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Copy link
Contributor

@jstoecker jstoecker left a comment

Choose a reason for hiding this comment

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

This isn't the right logic to filter adapters correctly, because you can't rely on name substrings to be consistent across hardware. Specifying a substring to match with the adapter description is only a convenience for users that want to select between adapters they've already enumerated and seen the description of. For example, on one of my desktops I get the following adapter descriptions:

Adapter[0]: NVIDIA GeForce RTX 4080 (SELECTED)
Adapter[1]: Intel(R) UHD Graphics 770
Adapter[2]: Microsoft Basic Render Driver

With your logic, it would try to find "NPU" and fail to match any adapter. Then it would try to find "GPU" and also fail to find any adapter. QC happens to use "GPU" and "NPU" in their adapter descriptions, but this isn't generically true.

If you want to the sample to prioritize NPU then you have to look for GENERIC_ML adapters that don't support graphics. Then, if that fails, you fall back to the first adapter that is at least supporting CORE_COMPUTE or GRAPHICS.

The logic in this sample already has fallback for D3D feature level, and you'd just want to modify it to also check for DXCORE_ADAPTER_ATTRIBUTE_D3D12_GRAPHICS or similar. You might reference this code too:
https://github.com/microsoft/DirectML/blob/master/Samples/DirectMLNpuInference/main.cpp

Modify DirectML_ESRGAN sample to by default try NPU first then retry
GPU.
@wonchung-microsoft
Copy link
Author

This isn't the right logic to filter adapters correctly, because you can't rely on name substrings to be consistent across hardware. Specifying a substring to match with the adapter description is only a convenience for users that want to select between adapters they've already enumerated and seen the description of. For example, on one of my desktops I get the following adapter descriptions:

Adapter[0]: NVIDIA GeForce RTX 4080 (SELECTED)
Adapter[1]: Intel(R) UHD Graphics 770
Adapter[2]: Microsoft Basic Render Driver

With your logic, it would try to find "NPU" and fail to match any adapter. Then it would try to find "GPU" and also fail to find any adapter. QC happens to use "GPU" and "NPU" in their adapter descriptions, but this isn't generically true.

If you want to the sample to prioritize NPU then you have to look for GENERIC_ML adapters that don't support graphics. Then, if that fails, you fall back to the first adapter that is at least supporting CORE_COMPUTE or GRAPHICS.

The logic in this sample already has fallback for D3D feature level, and you'd just want to modify it to also check for DXCORE_ADAPTER_ATTRIBUTE_D3D12_GRAPHICS or similar. You might reference this code too: https://github.com/microsoft/DirectML/blob/master/Samples/DirectMLNpuInference/main.cpp

@jstoecker I have made updates accordingly. Could you take a look? Thanks!

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