ARM-software / armnn

Arm NN ML Software. The code here is a read-only mirror of https://review.mlplatform.org/admin/repos/ml/armnn
https://developer.arm.com/products/processors/machine-learning/arm-nn
MIT License
1.14k stars 307 forks source link

ArmNN TfLite delegate does not pass down runtime options correctly #720

Closed brtal closed 8 months ago

brtal commented 1 year ago

The ArmNN Delegate class std::moves options to m_Options, but later references options directly in multiple places. While this leaves scalars in place, some of the underlying options are stored in containers (like std::vectors), which means you're no longer accessing the values you expect to access.

This blocks a handful of runtime options, including GPU tuning/level/MLGO tuning. IRuntime::Create is used with options that no longer contains these settings, and the delegate deoesn't pass down these options correctly to the backend.

The following patch fixes this issue:

diff --git a/delegate/src/armnn_delegate.cpp b/delegate/src/armnn_delegate.cpp
index 4e99335..a2a1527 100644
--- a/delegate/src/armnn_delegate.cpp
+++ b/delegate/src/armnn_delegate.cpp
@@ -136,13 +136,13 @@ Delegate::Delegate(armnnDelegate::DelegateOptions options)
     m_Options(std::move(options))
 {
     // Configures logging for ARMNN
-    if (options.IsLoggingEnabled())
+    if (m_Options.IsLoggingEnabled())
     {
-        armnn::ConfigureLogging(true, true, options.GetLoggingSeverity());
+        armnn::ConfigureLogging(true, true, m_Options.GetLoggingSeverity());
     }

     // Create ArmNN Runtime
-    m_Runtime = armnn::IRuntime::Create(options.GetRuntimeOptions());
+    m_Runtime = armnn::IRuntime::Create(m_Options.GetRuntimeOptions());

     std::vector<armnn::BackendId> backends;
     if (m_Runtime)

Thanks.

brtal commented 1 year ago

In general, we could use some guidance on how to correctly use the MLGO tuning options. I had to further patch the delegate to get the MLGO tuning file passed down correctly:

diff --git a/delegate/src/DelegateOptions.cpp b/delegate/src/DelegateOptions.cpp
index a55a57933..0ef272850 100644
--- a/delegate/src/DelegateOptions.cpp
+++ b/delegate/src/DelegateOptions.cpp
@@ -96,6 +96,8 @@ DelegateOptions::DelegateOptions(char const* const* options_keys,
         {
             armnn::BackendOptions option("GpuAcc", {{"MLGOTuningFilePath", std::string(options_values[i])}});
             optimizerOptions.m_ModelOptions.push_back(option);
+            // ClBackendContext parses backend options for this value, not model options.
+            runtimeOptions.m_BackendOptions.push_back(option);
         }
         else if (std::string(options_keys[i]) == std::string("gpu-tuning-file"))
         {

... however, this still doesn't work unless the MLGO tuning file path already exists. How is one supposed to create the tuning file?

Thanks!

TeresaARM commented 1 year ago

Hi @brtal

Thank you very much for sharing your findings.

The bug you describe in your first message was fixed in this patch: https://review.mlplatform.org/c/ml/armnn/+/8836 in December 2022, it is part of 23.02 release.

Regarding your second message, we would really love to review and merge your change, however the only method to accept your code is if you create a Gerrit review account at mlplatform.org and make your contribution there. You can use your GitHub credentials when creating your account. In this manner, we can use your suggested patch. The process for contributing to ArmNN is outlined in this Contributor Guide.

Regarding your question about the MLGOTuningFilePath, are you able to use the delegate without the file? the problem is that you cannot use the delegate because you see an error related with the file missing? if so, could you send us the error?

Please let us know if you find any issue.

Thank you very much. Kindest regards

MikeJKelly commented 8 months ago

Closed due to inactivity, if you need any more help please reopen the ticket or create a new one.

Best regards, Mike