-
Notifications
You must be signed in to change notification settings - Fork 298
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
For NPU Sample, add flexibility in device creation options and fix Generic ML Device logic #624
Merged
Zu-shi
merged 19 commits into
microsoft:master
from
Zu-shi:Resolve-NPU-Sample-Version-Update-Merge
Oct 22, 2024
Merged
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
ad40b43
For NPU Sample, add flexibility in device creation options and fix Ge…
Zu-shi e7d3bb3
Fix comment typo in NPU Sample
Zu-shi 1c6463e
Set D3D Feature Level Requirement to Generic, Optimize Adapter Selection
Zu-shi 7043b3a
Add flags to filter for GENERIC_ML only without COMPUTE
Zu-shi 3892f81
Improve labelling of flags & lists for developer friendliness
Zu-shi ba81019
Minor smaller PR feedback in comments & loop optimization
Zu-shi 55e7cdd
Merge pull request #1 from Zu-shi/Zu-shi-npu-sample-device-creation-p…
Zu-shi ad92847
address initial PR feedback
d9b710f
Simplify adapter discovery logic
afbf492
Update Package Versions, Simplify Device-Querying Logic
ad7d6fe
Address PR feedback
3632e30
Additional PR feedback
b97765b
Improve documentation to add context for NPU creation
7be18db
minor fix to sample output
6f872ef
Merge branch 'master' into Resolve-NPU-Sample-Version-Update-Merge
Zu-shi 344b17f
Address Initial PR feedback
7ec2b70
Create adapter under COMPUTE feature level if CORE adapters are not a…
8515d53
Merge branch 'master' into Resolve-NPU-Sample-Version-Update-Merge
f013bfd
Add comments noting that HMODULEs should be freed after usage
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<packages> | ||
<package id="Microsoft.AI.DirectML" version="1.15.2" targetFramework="native" /> | ||
<package id="Microsoft.AI.MachineLearning" version="1.17.0" targetFramework="native" /> | ||
<package id="Microsoft.AI.MachineLearning" version="1.19.2" targetFramework="native" /> | ||
<package id="Microsoft.Windows.ImplementationLibrary" version="1.0.220914.1" targetFramework="native" /> | ||
</packages> |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This is a preexisting issue, but all these HMODULES are leaking. Normally you should explicitly free these or use a smart pointer like wil::unique_hmodule: https://github.com/microsoft/wil/blob/6f60a1b76fb812c6af5db1bc7abdec0001edd43f/include/wil/resource.h#L2687.
You would need to refactor this sample to not have local variables owning the HMODULEs to really clean this up. Since this is an existing bug, I'm fine with completing this PR and addressing it in a subsequent change. Can you create an internal bug and maybe add a comment next to each LoadLibrary that this is for sample purposes only?
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.
Added comment and an internal bug assigned to me tracking this, thanks! :)