You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/05/19 23:17:37 UTC

[GitHub] [tvm] mbs-octoml opened a new pull request, #11382: Finish support for list-of-targets

mbs-octoml opened a new pull request, #11382:
URL: https://github.com/apache/tvm/pull/11382

   This finishes the work started in https://github.com/apache/tvm/pull/11173 to support
   'external codegen' targets in the N build-like API surfaces.
   
    - It turns out it's ok if a build is given only a single 'external codegen' target, so remove that check
      in CompilationConfig::Init. When Collage builds a 'candidate partition' it does so for a single target.
      As far as Collage is concerned it does not care whether the target is regular (eg Target("cuda")), or
      for a specific external codegen (eg Target("cutlass")), it just passes the target into the build.
   
    - Add CompilationConfig::FindPrimitiveTargetForKind which I'll later need to retrieve
      the external codegen Target instance corresponding to a "Compiler" attribute value.
   
    - Target.update_target_host_consist was supporting three API styles:
       - single target
       - map from device type to target
       - map from target to IRModule (for the ir_to_runtime API)
      I replaced all those calls with a more specialized 'canonicalize' call:
       - Target.canonicalize_target_and_host
       - Target.canonicalize_multi_targets_and_host
       - Target.canonicalize_target_map_and_host
      In particular, all the tuning interfaces (task extraction, tuning, tuning records) all explicitly
      *do not* support multiple targets since the underlying code just doesn't support that.
   


-- 
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: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mbs-octoml commented on a diff in pull request #11382: Finish support for list-of-targets

Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on code in PR #11382:
URL: https://github.com/apache/tvm/pull/11382#discussion_r878567855


##########
python/tvm/target/target.py:
##########
@@ -233,86 +233,95 @@ def canonicalize_target(target):
         return Target(target)
 
     @staticmethod
-    def canonicalize_multi_targets(multi_targets):
-        """Given a single or collection of target-like objects, returns a TVM Array of Target
-        objects representing then. Can convert from:
+    def canon_target_and_host(target, target_host=None):
+        """Returns a TVM Target capturing target and target_host. Also returns the host in
+        canonical form. The given target can be in any form recognized by
+        Target.canon_target. If given, target_host can be in any form recognized by
+        Target.canon_target. If target_host is given it will be set as the 'host' in the
+        result Target object (and a warning given).
+
+        Note that this method does not support heterogeneous compilation targets.
+        """
+        target = Target.canon_target(target)
+        target_host = Target.canon_target(target_host)
+        if target is None:
+            assert target_host is None, "Target host is not empty when target is empty."
+        if target_host is not None:
+            warnings.warn(
+                "target_host parameter is going to be deprecated. "
+                "Please pass in tvm.target.Target(target, host=target_host) instead."
+            )
+            target = Target(target, target_host)
+        if target is not None:
+            target_host = target.host
+        return target, target_host
+
+    @staticmethod
+    def canon_multi_target(multi_targets):
+        """Given a single target-like object, or a collection-like object of target-like objects,
+        returns a TVM Array of TVM Target objects representing then. Can convert from:
         - None (to None).
-        - A single target-like object in a form recognized by canonicalize_target.
+        - A single target-like object in a form recognized by canon_target.
         - A Python list or TVM Array of target-like objects in a form recognized by
-        canonicalize_target.
+        canon_target.
         - A Python dict or TVM Map from TVM IntImm objects representing device types to
-        a target-like object in a form recognized by canonicalize_target.
+        a target-like object in a form recognized by canon_target. (This is a legacy
+        method to represent heterogeneous targets. The keys are ignored.)
         """
         if multi_targets is None:
             return None
         if isinstance(multi_targets, (dict, Map)) and "kind" not in multi_targets:
             # Convert legacy heterogeneous map representation to ordinary list of targets.
-            return Target.canonicalize_multi_targets([t for _, t in multi_targets.items()])
+            return Target.canon_multi_target([t for _, t in multi_targets.items()])

Review Comment:
   done, thanks. My python is grade-school level.



-- 
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: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] SebastianBoblestETAS commented on a diff in pull request #11382: Finish support for list-of-targets

Posted by GitBox <gi...@apache.org>.
SebastianBoblestETAS commented on code in PR #11382:
URL: https://github.com/apache/tvm/pull/11382#discussion_r877784174


##########
tests/python/unittest/test_target_target.py:
##########
@@ -353,96 +353,111 @@ def test_target_with_host():
     assert tgt.host.attrs["registers_per_block"] == 32768
 
 
-def test_check_and_update_host_consist_0():
+def test_canon_target_and_host_0():
     target = None
     host = None
-    target, host = Target.check_and_update_host_consist(target, host)
+    target, host = Target.canon_target_and_host(target, host)
+    assert target is None
+    assert host is None
 
 
-def test_check_and_update_host_consist_1():
+def test_canon_target_and_host_1():
     target = None
     host = "llvm"
     with pytest.raises(AssertionError, match=r"Target host is not empty when target is empty."):
-        target, host = Target.check_and_update_host_consist(target, host)
+        target, host = Target.canon_target_and_host(target, host)
 
 
-def test_check_and_update_host_consist_2():
+def test_canon_target_and_host_2():
     target = Target("cuda")
     host = Target("llvm")
-    target, host = Target.check_and_update_host_consist(target, host)
+    target, host = Target.canon_target_and_host(target, host)
     assert target.kind.name == "cuda"
     assert target.host.kind.name == "llvm"
 
 
-def test_check_and_update_host_consist_3():
+def test_canon_target_and_host_3():
     target = Target(target="cuda", host="llvm")
     host = None
-    target, host = Target.check_and_update_host_consist(target, host)
+    target, host = Target.canon_target_and_host(target, host)
     assert target.kind.name == "cuda"
     assert target.host.kind.name == "llvm"
     assert host.kind.name == "llvm"
     assert target.host == host
 
 
-def test_check_and_update_host_consist_4():
-    """Test `check_and_update_host_consist` by using TVM Objects"""
-    cuda_device_type = tvm.device("cuda").device_type
-    target = {cuda_device_type: Target(target="cuda", host="llvm")}
-    host = None
-    target_1, host_1 = Target.check_and_update_host_consist(target, host)
-    assert isinstance(target_1, dict)
-    assert target_1[cuda_device_type].kind.name == "cuda"
-    assert target_1[cuda_device_type].host.kind.name == "llvm"
-    assert host_1 is None
-
-    target = {cuda_device_type: Target(tvm.runtime.container.String("cuda"))}
-    host = Target(tvm.runtime.container.String("llvm"))
-    target = tvm.runtime.convert(target)
-    assert isinstance(target, tvm.ir.container.Map)
-    target_2, host_2 = Target.check_and_update_host_consist(target, host)
-    assert isinstance(target_2, dict)
-    assert target_2[cuda_device_type].kind.name == "cuda"
-    assert host_2.kind.name == "llvm"
-
-
-def test_canonicalize_target_and_host_0():
+def test_canon_multi_target_and_host_0():
     with pytest.raises(AssertionError):
-        Target.canonicalize_target_and_host(None)
+        Target.canon_multi_target_and_host(None)
 
 
-def test_canonicalize_target_and_host_1():
-    raw_targets = Target.canonicalize_target_and_host({"kind": "llvm"})
+def test_canon_multi_target_and_host_1():
+    raw_targets = Target.canon_multi_target_and_host({"kind": "llvm"})
     assert len(raw_targets) == 1
     assert raw_targets[0].kind.name == "llvm"
 
 
-def test_canonicalize_target_and_host_2():
-    raw_targets = Target.canonicalize_target_and_host({1: "llvm", 2: "cuda"})
+def test_canon_multi_target_and_host_2():
+    raw_targets = Target.canon_multi_target_and_host({1: "llvm", 2: "cuda"})
     assert len(raw_targets) == 2
     assert raw_targets[0].kind.name == "llvm"
     assert raw_targets[1].kind.name == "cuda"
 
 
-def test_canonicalize_target_and_host_3():
-    raw_targets = Target.canonicalize_target_and_host(["llvm", "cuda"])
+def test_canon_multi_target_and_host_3():
+    raw_targets = Target.canon_multi_target_and_host(["llvm", "cuda"])
     assert len(raw_targets) == 2
     assert raw_targets[0].kind.name == "llvm"
     assert raw_targets[1].kind.name == "cuda"
 
 
-def test_canonicalize_target_and_host_4():
-    raw_targets = Target.canonicalize_target_and_host("llvm")
+def test_canon_multi_target_and_host_4():
+    raw_targets = Target.canon_multi_target_and_host("llvm")
     assert len(raw_targets) == 1
     assert raw_targets[0].kind.name == "llvm"
 
 
-def test_canonicalize_target_and_host_5():
-    raw_targets = Target.canonicalize_target_and_host("cuda", "llvm")
+def test_canon_multi_target_and_host_5():
+    raw_targets = Target.canon_multi_target_and_host("cuda", "llvm")
     assert len(raw_targets) == 1
     assert raw_targets[0].kind.name == "cuda"
     assert raw_targets[0].host.kind.name == "llvm"
 
 
+def test_canon_multi_target_and_host_6():

Review Comment:
   Have you thought about structuring tests into classes according to the function under test?
   Like 
   class TestCanonMultiTargetAndHost:
       def test_with_tvm_objects(self):
           # this test here
   
   The names of tests can then be shorter and more descriptive at the same time.
   This is more a question than a suggestion.
   
   



-- 
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: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mbs-octoml commented on a diff in pull request #11382: Finish support for list-of-targets

Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on code in PR #11382:
URL: https://github.com/apache/tvm/pull/11382#discussion_r878877868


##########
python/tvm/target/target.py:
##########
@@ -233,86 +233,95 @@ def canonicalize_target(target):
         return Target(target)
 
     @staticmethod
-    def canonicalize_multi_targets(multi_targets):
-        """Given a single or collection of target-like objects, returns a TVM Array of Target
-        objects representing then. Can convert from:
+    def canon_target_and_host(target, target_host=None):
+        """Returns a TVM Target capturing target and target_host. Also returns the host in
+        canonical form. The given target can be in any form recognized by
+        Target.canon_target. If given, target_host can be in any form recognized by
+        Target.canon_target. If target_host is given it will be set as the 'host' in the
+        result Target object (and a warning given).
+
+        Note that this method does not support heterogeneous compilation targets.
+        """
+        target = Target.canon_target(target)
+        target_host = Target.canon_target(target_host)
+        if target is None:
+            assert target_host is None, "Target host is not empty when target is empty."
+        if target_host is not None:
+            warnings.warn(
+                "target_host parameter is going to be deprecated. "
+                "Please pass in tvm.target.Target(target, host=target_host) instead."
+            )
+            target = Target(target, target_host)
+        if target is not None:
+            target_host = target.host
+        return target, target_host
+
+    @staticmethod
+    def canon_multi_target(multi_targets):
+        """Given a single target-like object, or a collection-like object of target-like objects,
+        returns a TVM Array of TVM Target objects representing then. Can convert from:
         - None (to None).
-        - A single target-like object in a form recognized by canonicalize_target.
+        - A single target-like object in a form recognized by canon_target.
         - A Python list or TVM Array of target-like objects in a form recognized by
-        canonicalize_target.
+        canon_target.
         - A Python dict or TVM Map from TVM IntImm objects representing device types to
-        a target-like object in a form recognized by canonicalize_target.
+        a target-like object in a form recognized by canon_target. (This is a legacy
+        method to represent heterogeneous targets. The keys are ignored.)
         """
         if multi_targets is None:
             return None
         if isinstance(multi_targets, (dict, Map)) and "kind" not in multi_targets:
             # Convert legacy heterogeneous map representation to ordinary list of targets.
-            return Target.canonicalize_multi_targets([t for _, t in multi_targets.items()])
+            return Target.canon_multi_target([t for _, t in multi_targets.items()])
         if isinstance(multi_targets, (list, Array)):
             # Multiple Target results.
-            return convert([Target.canonicalize_target(t) for t in multi_targets])
+            return convert([Target.canon_target(t) for t in multi_targets])
         # Single Target result.
-        return convert([Target.canonicalize_target(multi_targets)])
+        return convert([Target.canon_target(multi_targets)])
 
     @staticmethod
-    def canonicalize_target_and_host(target, target_host=None):
+    def canon_multi_target_and_host(target, target_host=None):
         """Returns a TVM Array<Target> capturing target and target_host. The given target can be in
-        any form recognized by Target.canonicalize_target or Target.canonicalize_multi_targets. If
-        given target_host can be in any form recognized by Target.canonicalize_target. If
-        target_host is given it will be set as the 'host' in each result Target object (and a
-        warning given).
+        any form recognized by Target.canon_multi_target. If given, target_host can be in
+        any form recognized by Target.canon_target. If target_host is given it will be set
+        as the 'host' in each result Target object (and a warning given).
         """
         # Convert target to Array<Target>, but not yet accounting for any host.
-        raw_targets = Target.canonicalize_multi_targets(target)
+        raw_targets = Target.canon_multi_target(target)
         assert raw_targets is not None
         # Convert host to Target, if given.
-        target_host = Target.canonicalize_target(target_host)
-        if target_host is None:
-            return raw_targets
-        warnings.warn(
-            "target_host parameter is going to be deprecated. "
-            "Please pass in tvm.target.Target(target, host=target_host) instead."
-        )
-        # Make sure the (canonical) host is captured in all the (canonical) targets.
-        return convert([Target(t, target_host) for t in raw_targets])
+        target_host = Target.canon_target(target_host)
+        if target_host is not None:
+            warnings.warn(
+                "target_host parameter is going to be deprecated. "
+                "Please pass in tvm.target.Target(target, host=target_host) instead."
+            )
+            # Make sure the (canonical) host is captured in all the (canonical) targets.
+            raw_targets = convert([Target(t, target_host) for t in raw_targets])
+        return raw_targets
 
     @staticmethod
-    def check_and_update_host_consist(target, host=None, target_is_dict_key=True):
-        """A helper function that merges a legacy "target, target_host" pair, then returns
-        the merged target and its host field. The function is for legacy target and target
-        host pair only, and should not be used in the new target system.
-
-        Parameters
-        ----------
-        target : Union[str, Dict[str, Any], Target]
-            The target or heterogeneous target
-        host : Union[str, Dict[str, Any], Target, None]
-            The target host
-        target_is_dict_key : Bool
-            When the type of target is dict, whether Target is the key (Otherwise the value)
-        """
-        if isinstance(target, (dict, str)):
-            target = convert(target)
-        if isinstance(host, (dict, str)):
-            host = convert(host)
-        if target is None:
-            assert host is None, "Target host is not empty when target is empty."
-            return target, host
-        if isinstance(target, Map) and "kind" not in target:
-            new_target = {}
-            for tgt, mod in target.items():
-                if not target_is_dict_key:
-                    tgt, mod = mod, tgt
-                if isinstance(tgt, (Map, String, Target)):
-                    tgt, host = Target.check_and_update_host_consist(tgt, host)
-                if not target_is_dict_key:
-                    tgt, mod = mod, tgt
-                new_target[tgt] = mod
-            target = new_target
-        else:
-            target = Target(target, host)
-            host = target.host
-        return target, host
+    def canon_target_map_and_host(target_map, target_host=None):
+        """Returns target_map as a map from TVM Target's in canonical form to IRModules. The keys
+        of the input target_map can be in any form recognized by Target.canon_target.
+        Similarly, if given, target_host can be in any form recognized by
+        Target.canon_target. The final target_map keys will capture the target_host in
+        canonical form. Also returns the target_host in canonical form."""
+        new_target_map = {}
+        if target_host is not None:
+            warnings.warn(
+                "target_host parameter is going to be deprecated. "
+                "Please pass in tvm.target.Target(target, host=target_host) instead."
+            )
+            target_host = Target.canon_target(target_host)
+        for tgt, mod in target_map.items():
+            tgt = Target.canon_target(tgt)
+            assert tgt is not None
+            if target_host is not None:
+                tgt = Target(tgt, target_host)
+            target_host = tgt.host

Review Comment:
   Actually I miss-spoke. The original code was also trying to handle the case when the first target already has a host, in which case it is copied into the rest. I'll preserve that behavior, fragile as it may be. Similarly, canon_target_and_host will return the host target even when it has already been provided in the target argument.



-- 
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: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] jwfromm merged pull request #11382: Finish support for list-of-targets

Posted by GitBox <gi...@apache.org>.
jwfromm merged PR #11382:
URL: https://github.com/apache/tvm/pull/11382


-- 
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: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] SebastianBoblestETAS commented on a diff in pull request #11382: Finish support for list-of-targets

Posted by GitBox <gi...@apache.org>.
SebastianBoblestETAS commented on code in PR #11382:
URL: https://github.com/apache/tvm/pull/11382#discussion_r877769322


##########
python/tvm/target/target.py:
##########
@@ -233,86 +233,95 @@ def canonicalize_target(target):
         return Target(target)
 
     @staticmethod
-    def canonicalize_multi_targets(multi_targets):
-        """Given a single or collection of target-like objects, returns a TVM Array of Target
-        objects representing then. Can convert from:
+    def canon_target_and_host(target, target_host=None):
+        """Returns a TVM Target capturing target and target_host. Also returns the host in
+        canonical form. The given target can be in any form recognized by
+        Target.canon_target. If given, target_host can be in any form recognized by
+        Target.canon_target. If target_host is given it will be set as the 'host' in the
+        result Target object (and a warning given).
+
+        Note that this method does not support heterogeneous compilation targets.
+        """
+        target = Target.canon_target(target)
+        target_host = Target.canon_target(target_host)
+        if target is None:
+            assert target_host is None, "Target host is not empty when target is empty."
+        if target_host is not None:
+            warnings.warn(
+                "target_host parameter is going to be deprecated. "
+                "Please pass in tvm.target.Target(target, host=target_host) instead."
+            )
+            target = Target(target, target_host)
+        if target is not None:
+            target_host = target.host
+        return target, target_host
+
+    @staticmethod
+    def canon_multi_target(multi_targets):
+        """Given a single target-like object, or a collection-like object of target-like objects,
+        returns a TVM Array of TVM Target objects representing then. Can convert from:
         - None (to None).
-        - A single target-like object in a form recognized by canonicalize_target.
+        - A single target-like object in a form recognized by canon_target.
         - A Python list or TVM Array of target-like objects in a form recognized by
-        canonicalize_target.
+        canon_target.
         - A Python dict or TVM Map from TVM IntImm objects representing device types to
-        a target-like object in a form recognized by canonicalize_target.
+        a target-like object in a form recognized by canon_target. (This is a legacy
+        method to represent heterogeneous targets. The keys are ignored.)
         """
         if multi_targets is None:
             return None
         if isinstance(multi_targets, (dict, Map)) and "kind" not in multi_targets:
             # Convert legacy heterogeneous map representation to ordinary list of targets.
-            return Target.canonicalize_multi_targets([t for _, t in multi_targets.items()])
+            return Target.canon_multi_target([t for _, t in multi_targets.items()])

Review Comment:
   ```suggestion
               return Target.canon_multi_target([t for t in multi_targets.values()])
   ```



-- 
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: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] SebastianBoblestETAS commented on a diff in pull request #11382: Finish support for list-of-targets

Posted by GitBox <gi...@apache.org>.
SebastianBoblestETAS commented on code in PR #11382:
URL: https://github.com/apache/tvm/pull/11382#discussion_r877774673


##########
python/tvm/target/target.py:
##########
@@ -233,86 +233,95 @@ def canonicalize_target(target):
         return Target(target)
 
     @staticmethod
-    def canonicalize_multi_targets(multi_targets):
-        """Given a single or collection of target-like objects, returns a TVM Array of Target
-        objects representing then. Can convert from:
+    def canon_target_and_host(target, target_host=None):
+        """Returns a TVM Target capturing target and target_host. Also returns the host in
+        canonical form. The given target can be in any form recognized by
+        Target.canon_target. If given, target_host can be in any form recognized by
+        Target.canon_target. If target_host is given it will be set as the 'host' in the
+        result Target object (and a warning given).
+
+        Note that this method does not support heterogeneous compilation targets.
+        """
+        target = Target.canon_target(target)
+        target_host = Target.canon_target(target_host)
+        if target is None:
+            assert target_host is None, "Target host is not empty when target is empty."
+        if target_host is not None:
+            warnings.warn(
+                "target_host parameter is going to be deprecated. "
+                "Please pass in tvm.target.Target(target, host=target_host) instead."
+            )
+            target = Target(target, target_host)
+        if target is not None:
+            target_host = target.host
+        return target, target_host
+
+    @staticmethod
+    def canon_multi_target(multi_targets):
+        """Given a single target-like object, or a collection-like object of target-like objects,
+        returns a TVM Array of TVM Target objects representing then. Can convert from:
         - None (to None).
-        - A single target-like object in a form recognized by canonicalize_target.
+        - A single target-like object in a form recognized by canon_target.
         - A Python list or TVM Array of target-like objects in a form recognized by
-        canonicalize_target.
+        canon_target.
         - A Python dict or TVM Map from TVM IntImm objects representing device types to
-        a target-like object in a form recognized by canonicalize_target.
+        a target-like object in a form recognized by canon_target. (This is a legacy
+        method to represent heterogeneous targets. The keys are ignored.)
         """
         if multi_targets is None:
             return None
         if isinstance(multi_targets, (dict, Map)) and "kind" not in multi_targets:
             # Convert legacy heterogeneous map representation to ordinary list of targets.
-            return Target.canonicalize_multi_targets([t for _, t in multi_targets.items()])
+            return Target.canon_multi_target([t for _, t in multi_targets.items()])
         if isinstance(multi_targets, (list, Array)):
             # Multiple Target results.
-            return convert([Target.canonicalize_target(t) for t in multi_targets])
+            return convert([Target.canon_target(t) for t in multi_targets])
         # Single Target result.
-        return convert([Target.canonicalize_target(multi_targets)])
+        return convert([Target.canon_target(multi_targets)])
 
     @staticmethod
-    def canonicalize_target_and_host(target, target_host=None):
+    def canon_multi_target_and_host(target, target_host=None):
         """Returns a TVM Array<Target> capturing target and target_host. The given target can be in
-        any form recognized by Target.canonicalize_target or Target.canonicalize_multi_targets. If
-        given target_host can be in any form recognized by Target.canonicalize_target. If
-        target_host is given it will be set as the 'host' in each result Target object (and a
-        warning given).
+        any form recognized by Target.canon_multi_target. If given, target_host can be in
+        any form recognized by Target.canon_target. If target_host is given it will be set
+        as the 'host' in each result Target object (and a warning given).
         """
         # Convert target to Array<Target>, but not yet accounting for any host.
-        raw_targets = Target.canonicalize_multi_targets(target)
+        raw_targets = Target.canon_multi_target(target)
         assert raw_targets is not None
         # Convert host to Target, if given.
-        target_host = Target.canonicalize_target(target_host)
-        if target_host is None:
-            return raw_targets
-        warnings.warn(
-            "target_host parameter is going to be deprecated. "
-            "Please pass in tvm.target.Target(target, host=target_host) instead."
-        )
-        # Make sure the (canonical) host is captured in all the (canonical) targets.
-        return convert([Target(t, target_host) for t in raw_targets])
+        target_host = Target.canon_target(target_host)
+        if target_host is not None:
+            warnings.warn(
+                "target_host parameter is going to be deprecated. "
+                "Please pass in tvm.target.Target(target, host=target_host) instead."
+            )
+            # Make sure the (canonical) host is captured in all the (canonical) targets.
+            raw_targets = convert([Target(t, target_host) for t in raw_targets])
+        return raw_targets
 
     @staticmethod
-    def check_and_update_host_consist(target, host=None, target_is_dict_key=True):
-        """A helper function that merges a legacy "target, target_host" pair, then returns
-        the merged target and its host field. The function is for legacy target and target
-        host pair only, and should not be used in the new target system.
-
-        Parameters
-        ----------
-        target : Union[str, Dict[str, Any], Target]
-            The target or heterogeneous target
-        host : Union[str, Dict[str, Any], Target, None]
-            The target host
-        target_is_dict_key : Bool
-            When the type of target is dict, whether Target is the key (Otherwise the value)
-        """
-        if isinstance(target, (dict, str)):
-            target = convert(target)
-        if isinstance(host, (dict, str)):
-            host = convert(host)
-        if target is None:
-            assert host is None, "Target host is not empty when target is empty."
-            return target, host
-        if isinstance(target, Map) and "kind" not in target:
-            new_target = {}
-            for tgt, mod in target.items():
-                if not target_is_dict_key:
-                    tgt, mod = mod, tgt
-                if isinstance(tgt, (Map, String, Target)):
-                    tgt, host = Target.check_and_update_host_consist(tgt, host)
-                if not target_is_dict_key:
-                    tgt, mod = mod, tgt
-                new_target[tgt] = mod
-            target = new_target
-        else:
-            target = Target(target, host)
-            host = target.host
-        return target, host
+    def canon_target_map_and_host(target_map, target_host=None):
+        """Returns target_map as a map from TVM Target's in canonical form to IRModules. The keys
+        of the input target_map can be in any form recognized by Target.canon_target.
+        Similarly, if given, target_host can be in any form recognized by
+        Target.canon_target. The final target_map keys will capture the target_host in
+        canonical form. Also returns the target_host in canonical form."""
+        new_target_map = {}
+        if target_host is not None:
+            warnings.warn(
+                "target_host parameter is going to be deprecated. "
+                "Please pass in tvm.target.Target(target, host=target_host) instead."
+            )
+            target_host = Target.canon_target(target_host)
+        for tgt, mod in target_map.items():
+            tgt = Target.canon_target(tgt)
+            assert tgt is not None
+            if target_host is not None:
+                tgt = Target(tgt, target_host)
+            target_host = tgt.host

Review Comment:
   I find it hard to understand what is happening here. If target_host is not None, it is assigned to the Target instance and then overwritten by the host attribute of that instance. And this happens for all elements in the dictionary.
   Can target_host change in any way here? Is line 322 really needed?



-- 
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: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] vinx13 commented on pull request #11382: Finish support for list-of-targets

Posted by GitBox <gi...@apache.org>.
vinx13 commented on PR #11382:
URL: https://github.com/apache/tvm/pull/11382#issuecomment-1136313439

   Hi @mbs-octoml with this PR I'm seeing some warnings `UserWarning: target_host parameter is going to be deprecated. Please pass in tvm.target.Target(target, host=target_host) instead.` even if I don't use target_host. Would you mind taking a look? It can be reproduced with the following script
   ```
   import tvm
   from tvm import te
   target = tvm.target.Target('nvidia/geforce-rtx-3070')
   N = 32
   A = te.placeholder(name='A', shape=[N])
   B = te.compute((N,), lambda i,: A[i] + 1.0)
   sch = te.create_schedule(B.op)
   sch[B].bind(sch[B].op.axis[0], te.thread_axis('threadIdx.x'))
   tvm.build(sch, [A, B], target)
   ```


-- 
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: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mbs-octoml commented on a diff in pull request #11382: Finish support for list-of-targets

Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on code in PR #11382:
URL: https://github.com/apache/tvm/pull/11382#discussion_r878568482


##########
tests/python/unittest/test_target_target.py:
##########
@@ -353,96 +353,111 @@ def test_target_with_host():
     assert tgt.host.attrs["registers_per_block"] == 32768
 
 
-def test_check_and_update_host_consist_0():
+def test_canon_target_and_host_0():
     target = None
     host = None
-    target, host = Target.check_and_update_host_consist(target, host)
+    target, host = Target.canon_target_and_host(target, host)
+    assert target is None
+    assert host is None
 
 
-def test_check_and_update_host_consist_1():
+def test_canon_target_and_host_1():
     target = None
     host = "llvm"
     with pytest.raises(AssertionError, match=r"Target host is not empty when target is empty."):
-        target, host = Target.check_and_update_host_consist(target, host)
+        target, host = Target.canon_target_and_host(target, host)
 
 
-def test_check_and_update_host_consist_2():
+def test_canon_target_and_host_2():
     target = Target("cuda")
     host = Target("llvm")
-    target, host = Target.check_and_update_host_consist(target, host)
+    target, host = Target.canon_target_and_host(target, host)
     assert target.kind.name == "cuda"
     assert target.host.kind.name == "llvm"
 
 
-def test_check_and_update_host_consist_3():
+def test_canon_target_and_host_3():
     target = Target(target="cuda", host="llvm")
     host = None
-    target, host = Target.check_and_update_host_consist(target, host)
+    target, host = Target.canon_target_and_host(target, host)
     assert target.kind.name == "cuda"
     assert target.host.kind.name == "llvm"
     assert host.kind.name == "llvm"
     assert target.host == host
 
 
-def test_check_and_update_host_consist_4():
-    """Test `check_and_update_host_consist` by using TVM Objects"""
-    cuda_device_type = tvm.device("cuda").device_type
-    target = {cuda_device_type: Target(target="cuda", host="llvm")}
-    host = None
-    target_1, host_1 = Target.check_and_update_host_consist(target, host)
-    assert isinstance(target_1, dict)
-    assert target_1[cuda_device_type].kind.name == "cuda"
-    assert target_1[cuda_device_type].host.kind.name == "llvm"
-    assert host_1 is None
-
-    target = {cuda_device_type: Target(tvm.runtime.container.String("cuda"))}
-    host = Target(tvm.runtime.container.String("llvm"))
-    target = tvm.runtime.convert(target)
-    assert isinstance(target, tvm.ir.container.Map)
-    target_2, host_2 = Target.check_and_update_host_consist(target, host)
-    assert isinstance(target_2, dict)
-    assert target_2[cuda_device_type].kind.name == "cuda"
-    assert host_2.kind.name == "llvm"
-
-
-def test_canonicalize_target_and_host_0():
+def test_canon_multi_target_and_host_0():
     with pytest.raises(AssertionError):
-        Target.canonicalize_target_and_host(None)
+        Target.canon_multi_target_and_host(None)
 
 
-def test_canonicalize_target_and_host_1():
-    raw_targets = Target.canonicalize_target_and_host({"kind": "llvm"})
+def test_canon_multi_target_and_host_1():
+    raw_targets = Target.canon_multi_target_and_host({"kind": "llvm"})
     assert len(raw_targets) == 1
     assert raw_targets[0].kind.name == "llvm"
 
 
-def test_canonicalize_target_and_host_2():
-    raw_targets = Target.canonicalize_target_and_host({1: "llvm", 2: "cuda"})
+def test_canon_multi_target_and_host_2():
+    raw_targets = Target.canon_multi_target_and_host({1: "llvm", 2: "cuda"})
     assert len(raw_targets) == 2
     assert raw_targets[0].kind.name == "llvm"
     assert raw_targets[1].kind.name == "cuda"
 
 
-def test_canonicalize_target_and_host_3():
-    raw_targets = Target.canonicalize_target_and_host(["llvm", "cuda"])
+def test_canon_multi_target_and_host_3():
+    raw_targets = Target.canon_multi_target_and_host(["llvm", "cuda"])
     assert len(raw_targets) == 2
     assert raw_targets[0].kind.name == "llvm"
     assert raw_targets[1].kind.name == "cuda"
 
 
-def test_canonicalize_target_and_host_4():
-    raw_targets = Target.canonicalize_target_and_host("llvm")
+def test_canon_multi_target_and_host_4():
+    raw_targets = Target.canon_multi_target_and_host("llvm")
     assert len(raw_targets) == 1
     assert raw_targets[0].kind.name == "llvm"
 
 
-def test_canonicalize_target_and_host_5():
-    raw_targets = Target.canonicalize_target_and_host("cuda", "llvm")
+def test_canon_multi_target_and_host_5():
+    raw_targets = Target.canon_multi_target_and_host("cuda", "llvm")
     assert len(raw_targets) == 1
     assert raw_targets[0].kind.name == "cuda"
     assert raw_targets[0].host.kind.name == "llvm"
 
 
+def test_canon_multi_target_and_host_6():

Review Comment:
   Not really experienced with good pytest style, just mimicking whatever I happen to find already in place. Happy to try that if it's considered the preferred idiom? 



-- 
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: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mbs-octoml commented on a diff in pull request #11382: Finish support for list-of-targets

Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on code in PR #11382:
URL: https://github.com/apache/tvm/pull/11382#discussion_r878571872


##########
python/tvm/target/target.py:
##########
@@ -233,86 +233,95 @@ def canonicalize_target(target):
         return Target(target)
 
     @staticmethod
-    def canonicalize_multi_targets(multi_targets):
-        """Given a single or collection of target-like objects, returns a TVM Array of Target
-        objects representing then. Can convert from:
+    def canon_target_and_host(target, target_host=None):
+        """Returns a TVM Target capturing target and target_host. Also returns the host in
+        canonical form. The given target can be in any form recognized by
+        Target.canon_target. If given, target_host can be in any form recognized by
+        Target.canon_target. If target_host is given it will be set as the 'host' in the
+        result Target object (and a warning given).
+
+        Note that this method does not support heterogeneous compilation targets.
+        """
+        target = Target.canon_target(target)
+        target_host = Target.canon_target(target_host)
+        if target is None:
+            assert target_host is None, "Target host is not empty when target is empty."
+        if target_host is not None:
+            warnings.warn(
+                "target_host parameter is going to be deprecated. "
+                "Please pass in tvm.target.Target(target, host=target_host) instead."
+            )
+            target = Target(target, target_host)
+        if target is not None:
+            target_host = target.host
+        return target, target_host
+
+    @staticmethod
+    def canon_multi_target(multi_targets):
+        """Given a single target-like object, or a collection-like object of target-like objects,
+        returns a TVM Array of TVM Target objects representing then. Can convert from:
         - None (to None).
-        - A single target-like object in a form recognized by canonicalize_target.
+        - A single target-like object in a form recognized by canon_target.
         - A Python list or TVM Array of target-like objects in a form recognized by
-        canonicalize_target.
+        canon_target.
         - A Python dict or TVM Map from TVM IntImm objects representing device types to
-        a target-like object in a form recognized by canonicalize_target.
+        a target-like object in a form recognized by canon_target. (This is a legacy
+        method to represent heterogeneous targets. The keys are ignored.)
         """
         if multi_targets is None:
             return None
         if isinstance(multi_targets, (dict, Map)) and "kind" not in multi_targets:
             # Convert legacy heterogeneous map representation to ordinary list of targets.
-            return Target.canonicalize_multi_targets([t for _, t in multi_targets.items()])
+            return Target.canon_multi_target([t for _, t in multi_targets.items()])
         if isinstance(multi_targets, (list, Array)):
             # Multiple Target results.
-            return convert([Target.canonicalize_target(t) for t in multi_targets])
+            return convert([Target.canon_target(t) for t in multi_targets])
         # Single Target result.
-        return convert([Target.canonicalize_target(multi_targets)])
+        return convert([Target.canon_target(multi_targets)])
 
     @staticmethod
-    def canonicalize_target_and_host(target, target_host=None):
+    def canon_multi_target_and_host(target, target_host=None):
         """Returns a TVM Array<Target> capturing target and target_host. The given target can be in
-        any form recognized by Target.canonicalize_target or Target.canonicalize_multi_targets. If
-        given target_host can be in any form recognized by Target.canonicalize_target. If
-        target_host is given it will be set as the 'host' in each result Target object (and a
-        warning given).
+        any form recognized by Target.canon_multi_target. If given, target_host can be in
+        any form recognized by Target.canon_target. If target_host is given it will be set
+        as the 'host' in each result Target object (and a warning given).
         """
         # Convert target to Array<Target>, but not yet accounting for any host.
-        raw_targets = Target.canonicalize_multi_targets(target)
+        raw_targets = Target.canon_multi_target(target)
         assert raw_targets is not None
         # Convert host to Target, if given.
-        target_host = Target.canonicalize_target(target_host)
-        if target_host is None:
-            return raw_targets
-        warnings.warn(
-            "target_host parameter is going to be deprecated. "
-            "Please pass in tvm.target.Target(target, host=target_host) instead."
-        )
-        # Make sure the (canonical) host is captured in all the (canonical) targets.
-        return convert([Target(t, target_host) for t in raw_targets])
+        target_host = Target.canon_target(target_host)
+        if target_host is not None:
+            warnings.warn(
+                "target_host parameter is going to be deprecated. "
+                "Please pass in tvm.target.Target(target, host=target_host) instead."
+            )
+            # Make sure the (canonical) host is captured in all the (canonical) targets.
+            raw_targets = convert([Target(t, target_host) for t in raw_targets])
+        return raw_targets
 
     @staticmethod
-    def check_and_update_host_consist(target, host=None, target_is_dict_key=True):
-        """A helper function that merges a legacy "target, target_host" pair, then returns
-        the merged target and its host field. The function is for legacy target and target
-        host pair only, and should not be used in the new target system.
-
-        Parameters
-        ----------
-        target : Union[str, Dict[str, Any], Target]
-            The target or heterogeneous target
-        host : Union[str, Dict[str, Any], Target, None]
-            The target host
-        target_is_dict_key : Bool
-            When the type of target is dict, whether Target is the key (Otherwise the value)
-        """
-        if isinstance(target, (dict, str)):
-            target = convert(target)
-        if isinstance(host, (dict, str)):
-            host = convert(host)
-        if target is None:
-            assert host is None, "Target host is not empty when target is empty."
-            return target, host
-        if isinstance(target, Map) and "kind" not in target:
-            new_target = {}
-            for tgt, mod in target.items():
-                if not target_is_dict_key:
-                    tgt, mod = mod, tgt
-                if isinstance(tgt, (Map, String, Target)):
-                    tgt, host = Target.check_and_update_host_consist(tgt, host)
-                if not target_is_dict_key:
-                    tgt, mod = mod, tgt
-                new_target[tgt] = mod
-            target = new_target
-        else:
-            target = Target(target, host)
-            host = target.host
-        return target, host
+    def canon_target_map_and_host(target_map, target_host=None):
+        """Returns target_map as a map from TVM Target's in canonical form to IRModules. The keys
+        of the input target_map can be in any form recognized by Target.canon_target.
+        Similarly, if given, target_host can be in any form recognized by
+        Target.canon_target. The final target_map keys will capture the target_host in
+        canonical form. Also returns the target_host in canonical form."""
+        new_target_map = {}
+        if target_host is not None:
+            warnings.warn(
+                "target_host parameter is going to be deprecated. "
+                "Please pass in tvm.target.Target(target, host=target_host) instead."
+            )
+            target_host = Target.canon_target(target_host)
+        for tgt, mod in target_map.items():
+            tgt = Target.canon_target(tgt)
+            assert tgt is not None
+            if target_host is not None:
+                tgt = Target(tgt, target_host)
+            target_host = tgt.host

Review Comment:
   That's left over from line 307 of the original, but it (and the similar code in canon_target_and_host) is not necessary since by this point target_host has been canonicalized. Nuked, thx.



-- 
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: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mbs-octoml commented on pull request #11382: Finish support for list-of-targets

Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on PR #11382:
URL: https://github.com/apache/tvm/pull/11382#issuecomment-1137594484

   I think we can fix that, i'll fold it into the next pr.


-- 
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: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org