Mousius commented on a change in pull request #8300:
URL: https://github.com/apache/tvm/pull/8300#discussion_r691257639



##########
File path: python/tvm/relay/op/contrib/arm_compute_lib.py
##########
@@ -69,7 +69,7 @@ def partition_for_arm_compute_lib(mod, params=None, **opts):
         ]
     )
 
-    return seq(mod)
+    return seq(mod), None

Review comment:
       It appears TVM uses the Numpy-esque docstrings where multiple return 
values are documented as lists rather than a combined value 
https://github.com/apache/tvm/blob/main/python/tvm/relay/build_module.py#L140-L150,
 could you match this style?

##########
File path: src/relay/backend/contrib/ethosn/codegen_ethosn.h
##########
@@ -227,7 +227,9 @@ NetworkWithIDs ConstructNetwork(const IRModule& mod, const 
GlobalVar& var, const
 /*! \brief Attributes to store the compiler options for Ethos-N */
 struct EthosnCompilerConfigNode : public 
tvm::AttrsNode<EthosnCompilerConfigNode> {
   String variant;
-  int sram_size_bytes;
+  String sram_size;

Review comment:
       It'd be better to have these typed correctly as they're passed in and 
then recast to string at the boundary with the driver stack.

##########
File path: python/tvm/relay/op/contrib/ethosn.py
##########
@@ -61,6 +61,53 @@ def partition_for_ethosn(mod, params=None, **opts):
     -------
     ret : annotated and partitioned module.
     """
+    if opts:
+        tops = opts.get("tops", None)
+        ple_ratio = opts.get("ple_ratio", None)
+        sram_size = opts.get("sram_size", None)
+        if tops or ple_ratio or sram_size:
+            raise ValueError(
+                "Setting tops, ple_ratio or sram_size has no effect when 
targeting Ethos-N77"
+            )
+
+    if params:
+        mod["main"] = bind_params_by_name(mod["main"], params)
+
+    seq = tvm.transform.Sequential(
+        [
+            transform.InferType(),
+            transform.MergeComposite(pattern_table()),
+            transform.AnnotateTarget("ethos-n"),
+            transform.MergeCompilerRegions(),
+            transform.PartitionGraph(),
+        ]
+    )
+
+    return seq(mod), None
+
+
+def partition_for_ethosn78(mod, params=None, **opts):
+    """Partition the graph greedily offloading supported
+    operators to Arm Ethos-N NPU.
+
+    Parameters
+    ----------
+    mod : Module
+        The module to run passes on.
+    params : Optional[Dict[str, NDArray]]
+        Constant input parameters.
+
+    Returns
+    -------
+    ret : annotated and partitioned module.

Review comment:
       I don't think this docstring got updated?

##########
File path: python/tvm/relay/op/contrib/ethosn.py
##########
@@ -61,6 +61,53 @@ def partition_for_ethosn(mod, params=None, **opts):
     -------
     ret : annotated and partitioned module.
     """
+    if opts:
+        tops = opts.get("tops", None)
+        ple_ratio = opts.get("ple_ratio", None)
+        sram_size = opts.get("sram_size", None)
+        if tops or ple_ratio or sram_size:
+            raise ValueError(
+                "Setting tops, ple_ratio or sram_size has no effect when 
targeting Ethos-N77"
+            )
+
+    if params:
+        mod["main"] = bind_params_by_name(mod["main"], params)
+
+    seq = tvm.transform.Sequential(
+        [
+            transform.InferType(),
+            transform.MergeComposite(pattern_table()),
+            transform.AnnotateTarget("ethos-n"),
+            transform.MergeCompilerRegions(),
+            transform.PartitionGraph(),
+        ]
+    )
+
+    return seq(mod), None
+
+
+def partition_for_ethosn78(mod, params=None, **opts):
+    """Partition the graph greedily offloading supported
+    operators to Arm Ethos-N NPU.
+
+    Parameters
+    ----------
+    mod : Module
+        The module to run passes on.
+    params : Optional[Dict[str, NDArray]]
+        Constant input parameters.
+
+    Returns
+    -------
+    ret : annotated and partitioned module.
+    """
+    config = opts
+    if not config:
+        config = {}
+
+    if config.get("variant", "").lower() != "ethos-n78":

Review comment:
       Apologies, I can clarify what I meant, if we take the scenarios in which 
this line of code is ran we'll have:
   
   1. Variant isn't set
      * Defaults to `""` 
      * `"" != "Ethos-N78"`
      * Sets to `Ethos-N78`
   
   2. Variant is set, but not to `Ethos-N78` (we'll use `woofles`)
      * Gets value `"woofles"` 
      * `"woofles" != "Ethos-N78"`
      * Sets to `Ethos-N78`
   
   3. Variant is set, to `Ethos-N78`
      * Remains as `Ethos-N78`
   
   So I think in all cases it ends up set to `Ethos-N78`, casing appears to be 
irrelevant as the final value can be any casing if it lowers to `ethos-n78`. 
Let me know if I've missed anything here!
   
   
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to