Added Arm NN Settings for Arm NN delegate #60352
Conversation
| message ArmNNSettings { | ||
| optional string backends = 1; | ||
| optional bool fastmath = 2; | ||
| optional string additional_parameters = 3; |
There was a problem hiding this comment.
Thanks for raising this CL!
I understand that you added a link to the Arm NN delegate options on line 628. I think it would nevertheless be useful to have a brief description of the fields and their meaning in this file too. That's also what we have for the other delegate settings in this file.
Would you consider adding this?
| @@ -622,6 +624,14 @@ message CPUSettings { | |||
| optional int32 num_threads = 1 [default = -1]; | |||
| } | |||
|
|
|||
| // Arm NN Delegate Settings | |||
| // More information about Arm NN delegate options can be found in https://arm-software.github.io/armnn/latest/delegate.xhtml | |||
There was a problem hiding this comment.
Wrap long line to 80 columns?
| @@ -622,6 +624,14 @@ message CPUSettings { | |||
| optional int32 num_threads = 1 [default = -1]; | |||
| } | |||
|
|
|||
| // Arm NN Delegate Settings | |||
There was a problem hiding this comment.
Hi. I noticed a very tiny issue in your comment here that I'd like to bring to your attention. In this specific block of text, I noticed that there's a missing full stop at the end of a sentence. I understand that this is a minor issue, but adding the full stop would help improve the readability and clarity of the text.
The Google style guide recommends including punctuation in comments https://google.github.io/styleguide/cppguide.html#Punctuation,_Spelling_and_Grammar.
Would you be willing to add it, or do you have any thoughts on this matter?
| // More information about Arm NN delegate options can be found in https://arm-software.github.io/armnn/latest/delegate.xhtml | ||
| message ArmNNSettings { | ||
| optional string backends = 1; | ||
| optional bool fastmath = 2; |
There was a problem hiding this comment.
The current documentation here does not make the following things clear:
(1) What effect does this fastmath field have?
(2) What is the relationship between this field and the "enable-fast-math" parameter in additional_parameters?
E.g. if this field is set to true, but "enable-fast-math" is set to "false", which takes precedence?
There was a problem hiding this comment.
Our preference was to have a single string of key value pairs exactly the same as for our existing delegate as the configuration specification but the feedback we got was that a higher degree of structure was required so our compromise proposal was to expose the two settings we would ideally like the Acceleration Service to have access to.... so fastmath in the message and 'enable-fast-math' are the same setting... the one in the message takes precedence
There was a problem hiding this comment.
We will update our documentation to make this clear to users of the Opaque Delegate
| // Arm NN Delegate Settings | ||
| // More information about Arm NN delegate options can be found in https://arm-software.github.io/armnn/latest/delegate.xhtml | ||
| message ArmNNSettings { | ||
| optional string backends = 1; |
There was a problem hiding this comment.
Please document this field.
E.g.
// List of ARM NN backends, in order of preference, that will used to process the model.
// If all or part of the model is not supported by a backend the next in order will be tried.
// Valid backend ids include:
// - "CpuAcc": the CPU backend.
// - "CpuRef": the CPU reference kernels.
// - "GpuAcc": the GPU backend.
// - "EthosNAcc": the Ethos-N NPU backend.
| // More information about Arm NN delegate options can be found in https://arm-software.github.io/armnn/latest/delegate.xhtml | ||
| message ArmNNSettings { | ||
| optional string backends = 1; | ||
| optional bool fastmath = 2; |
There was a problem hiding this comment.
Please document this field.
| message ArmNNSettings { | ||
| optional string backends = 1; | ||
| optional bool fastmath = 2; | ||
| optional string additional_parameters = 3; |
There was a problem hiding this comment.
Please document this field.
E.g.
// Comma-separated list of name=value pairs, specifying additional ARM NN runtime options,
// e.g. "number-of-threads=4,gpu-tuning-level=3,enable-fast-math=true,logging-severity=Debug".
// See https://arm-software.github.io/armnn/latest/runtimeoptions.xhtml.
// Note that other fields in ArmNNSettings will take precedence over any corresponding parameters
// specified here, if both are set.
google-ml-butler
bot
added
kokoro:force-run
|
Hello @alankelly, We would highly appreciate your review. Thank you very much. |
|
@sirakiin Can you PTAL at this, I am unfamiliar with the delegate interface. Thanks! |
|
Would this be possible to be merged? Thank you very much. |
copybara-service
bot
merged commit 9b60f14
into
tensorflow:master
Added Arm NN settings to configuration.proto and related files for Arm NN delegate