-
Notifications
You must be signed in to change notification settings - Fork 755
Add simple_tensor_and_data_graph sample #1394
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
base: main
Are you sure you want to change the base?
Add simple_tensor_and_data_graph sample #1394
Conversation
* Added initial tensor and data graph sample, common framework and documentation. Signed-off-by: Aron Virginas-Tar <aron.virginas-tar@arm.com>
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.
Some first comments...
...tensions/tensor_and_data_graph/simple_tensor_and_data_graph/simple_tensor_and_data_graph.cpp
Outdated
Show resolved
Hide resolved
samples/extensions/tensor_and_data_graph/tensor_and_data_graph_common.cpp
Show resolved
Hide resolved
samples/extensions/tensor_and_data_graph/tensor_and_data_graph_common.cpp
Show resolved
Hide resolved
samples/extensions/tensor_and_data_graph/tensor_and_data_graph_common.cpp
Show resolved
Hide resolved
samples/extensions/tensor_and_data_graph/tensor_and_data_graph_common.cpp
Show resolved
Hide resolved
samples/extensions/tensor_and_data_graph/tensor_and_data_graph_common.cpp
Outdated
Show resolved
Hide resolved
samples/extensions/tensor_and_data_graph/tensor_and_data_graph_common.cpp
Outdated
Show resolved
Hide resolved
samples/extensions/tensor_and_data_graph/tensor_and_data_graph_common.cpp
Outdated
Show resolved
Hide resolved
samples/extensions/tensor_and_data_graph/tensor_and_data_graph_common.cpp
Outdated
Show resolved
Hide resolved
samples/extensions/tensor_and_data_graph/tensor_and_data_graph_common.h
Outdated
Show resolved
Hide resolved
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.
One more comment...
samples/extensions/tensor_and_data_graph/simple_tensor_and_data_graph/README.adoc
Outdated
Show resolved
Hide resolved
1095425
to
7631290
Compare
* Fixed issues highlighted by reviewers * Fixed copyright headers Signed-off-by: Aron Virginas-Tar <aron.virginas-tar@arm.com>
7631290
to
4e46f43
Compare
It seems the script checking the copyright headers expects all copyright lines to be upgraded to the current year (2025), not just the one referring to the contributor making the latest changes (Arm). In consequence, the following header will result in failure:
In effect the script is expecting us to update NVIDIA's copyright line to 2025 as well. I'm wondering if this makes sense, or whether it is a bug in the copyright checker script. Can you please confirm? |
Signed-off-by: Aron Virginas-Tar <aron.virginas-tar@arm.com>
Thanks for clarifying this, I have now updated the copyright headers according to what the CI script expects. |
e2c3979
to
4db21e2
Compare
Signed-off-by: Aron Virginas-Tar <aron.virginas-tar@arm.com>
@asuessenbach Clang-format still fails in CI but passes locally. I suspect it is due to a mismatch in clang-format versions between my machine and CI, as you also suggested. If you're happy with what the code looks like, I would propose to ignore this check and get the PR merged nonetheless. |
Apparently, there's more than just the format check failing: on ios, the release build is failing as well. |
The iOS issue is caused by the CI using an old version of the SDK where the shader compiler is lacking required extensions. I encountered the same issues with my Slang shaders PRs and had to disable shader compilation in CI on that platform. That's something we need to fix separately. |
Currently we do not support iOS and macOS – even though the build succeeds on macOS – as running the sample will not be possible via MoltenVK . Since for now we have to rely on the ML Emulation Layer for Vulkan to execute this sample, we can only support the platforms that are able to run the Emulation Layer, namely Windows and Linux, with Android support being experimental. All this is mentioned in the readme. |
Then you could as well disable the sample on Mac/iOS. We do that for some of the samples, e.g. the OpenCL ARM sample that only works on Android. This is done at CMake level, see https://github.com/KhronosGroup/Vulkan-Samples/blob/main/samples/extensions/open_cl_interop_arm/CMakeLists.txt as an example. |
Signed-off-by: Aron Virginas-Tar <aron.virginas-tar@arm.com>
Signed-off-by: Aron Virginas-Tar <aron.virginas-tar@arm.com>
Hi all, Thank you for all your comments so far. We really appreciate it. I believe Aron has sorted the issues mentioned above and all checks are now passing. If possible could we get a few more reviewers to take a look please? If you would like to try out this new sample locally, we provide an emulation layer which allows them to run. This can be found here: https://github.com/arm/ai-ml-emulation-layer-for-vulkan There is also a pre-built Windows package available here: https://www.arm.com/-/media/Files/developer/MLEmulationLayerForVulkan or you can grab this through our tutorial: https://learn.arm.com/learning-paths/mobile-graphics-and-gaming/vulkan-ml-sample/. This tutorial is actually for this first sample in this PR, so it shows you everything needed to try it out. If you do have any questions feel free to reach out. The guide and pre-built is only for Windows though, it does work on Linux, but this will require a manual build from the repository (https://github.com/arm/ai-ml-emulation-layer-for-vulkan). See the build guide for this. Lastly, we have also had to update the vulkan-headers and volk versions to 1.4.321.0, as support for these extensions was only added recently. Let me know your thoughts on this? Thanks, Matthew |
I'll do a review before the next sample meeting. |
The samples in this folder are: | ||
|
||
* xref:simple_tensor_and_data_graph/README.adoc[Simple tensor and data graph] | ||
* xref:graph_constants/README.adoc[Graph constants] |
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.
None of these folders (except the first) exists (yet). Might be better to remove and add back once the (yet) missing samples land.
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 agree, I have removed these and will add them back in follow up samples. Thank you!
|
||
Alternatively, on Windows, the Vulkan Configurator tool — which is part of the Vulkan SDK — can also be used, as described below. If it is not already installed, it can be obtained from the link:https://vulkan.lunarg.com/[Vulkan SDK] website. | ||
|
||
Once installed, open Vulkan Configurator and navigate to the Vulkan Layers Location tab. Click the plus (+) symbol to append a user-defined layers path, |
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.
Should add a note that the validation layers need to be disabled, otherwise the sample will crash as the layers do not (yet) know about the new extension. At least that was the case for me.
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.
We haven't encountered this so far, but it's definitely no harm adding it. Thank you for letting us know.
@@ -0,0 +1,589 @@ | |||
:source-highlighter: coderay |
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 don't think this will work with the Antora site build and needs to be removed.
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.
Removed, thank you!
|
||
Note: It is recommended to disable the validation layer when running these samples as the new extensions are not supported yet. | ||
|
||
image::loading_layers.png[Loading Layers Completed] |
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.
Needs to be changed to
image::./loading_layers.png[Loading Layers Completed]
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, thanks!
|
||
This should add the layers to the Vulkan Loader Management tab. The Graph layer needs to sit on top of the Tensor layer. | ||
|
||
image::verify_layers.png[VK Layers] |
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.
Needs to be changed to
image::./verify_layers.png[VK Layers]
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, thanks!
|
||
== Conclusion | ||
|
||
In this sample we've introduced the VK_ARM_tensors and VK_ARM_data_graph extensions and shown how to use them to run a simple neural network. We've shown the steps needed to run a neural network - creating a pipeline layout, data graph pipeline, data graph pipeline session, tensor objects, descriptor sets and finally recording binding and dispatch commands in a command buffer. The following samples in xref:..\README.adoc[this series] build upon these concepts. |
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.
Link needs to be changed to
xref:samples/extensions/tensor_and_data_graph/README.adoc[this series]
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, thank you!
You also should add the simple tensor sample as a child item to the tensor samples in the nav.adoc |
Btw. the sample itself runs fine on my setup using the emulation layers. So once the documentation things are fixed, this is good to go (from my side). |
That's great, thanks Sascha for reviewing this! I had some trouble trying to build the docs site myself, is there is a guide or documentation on how to do this? I have created a patch anyway with the changes you suggested above, which hopefully will cover it. |
Sadly not. I do have my own docs site testbed that's easier to build (due to not including the spec). Might be a good idea to offer something like that with an easy to use explanation to all maintainers of the samples. |
Copyright header check fails in |
Signed-off-by: Aron Virginas-Tar <aron.virginas-tar@arm.com>
30fcd0a
to
aad840f
Compare
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 eventually got far enough that the sample just reports "[error] Required device extension VK_KHR_deferred_host_operations not available, cannot run"
, so that's as far as I can go I'm afraid.
Are you trying to run the sample on macOS? MoltenVK does not seem to fully support |
No, it's our own hardware, which doesn't support VK_KHR_deferred_host_operations. I'm really just saying that I can't test this. |
VK_ARM_data_graph depends on |
Signed-off-by: Matthew Sloyan <matthew.sloyan@arm.com>
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 did a small amount of work to make VK_KHR_deferred_host_operations appear available on our platform. I then see an assert in our driver where the compiler is seeing 64-bit integers being used.
It looks like the emulation layer is using 64-bit integers in shaders. Those 64-bit extensions should probably be added to this sample so that it gracefully exits as unsupported if they aren't available.
Signed-off-by: Matthew Sloyan <matthew.sloyan@arm.com>
Thanks @gary-sweet for the suggestion and for trying this out, I have added a shaderInt64 check to the request_gpu_features function, hopefully that is what you meant. |
Hi all, Thanks for all the reviews! @gary-sweet I am not sure if there is anything to do on my side to address the change requested, I pushed a patch to fix it, but, possibly it needs your input? Also would you be happy to approve based on your code review? However, I know you were blocked due to the VK_KHR_deferred_host_operations dependency. No worries if not. @asuessenbach I believe you had said you wouldn't have time to try out the sample which is no problem at all, but would you also be happy to approve based on the code review you conducted? Again no worries if not. Lastly, @SaschaWillems thanks again for trying this out and reviewing it. Your approval has been removed due to the changes above, would it be possible to add this again if you don't mind? If anyone else does want to try it out please see my comment above: #1394 (comment) Kind regards, Matthew |
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.
The best that I can do is to remove my "Request Changes" sadly. I can't run any more of this so I won't be able to approve it myself.
I tried to run this sample using the emulation layer as described in https://github.com/arm/ai-ml-emulation-layer-for-vulkan, but failed. I've copied the dll- and json-files from MLEmulationLayerForVulkan.zip into some directory, set the environment in VS to VK_ADD_LAYER_PATH="the\path\to\the\layers" But there's still no "VK_ARM_tensors" extension supported. Anything else I need to do, to get that? |
Thanks @asuessenbach for trying this out. Hmm I have a feeling it's the Vulkan SDK version that's being used. Are you using 1.4.321 by any chance? |
NOTE: This is essentially the same code as in the previously closed PR no. 1381, but the sample (and thus the branch) was renamed.
Description
This sample demonstrates the usage of the
VK_ARM_tensors
andVK_ARM_data_graph
extensions for machine learning inference. These allow you to define a neural network with TOSA operations and then execute inferences using hardware acceleration.This is the first in a series of samples which introduce the tensor and graph extensions.
General Checklist:
Please ensure the following points are checked:
Note: The Samples CI runs a number of checks including:
If this PR contains framework changes:
batch
command line argument to make sure all samples still work properlySample Checklist
If your PR contains a new or modified sample, these further checks must be carried out in addition to the General Checklist: