tqchen commented on a change in pull request #5838:
URL: https://github.com/apache/incubator-tvm/pull/5838#discussion_r443085167



##########
File path: src/target/target_id.cc
##########
@@ -91,38 +91,74 @@ void VerifyTypeInfo(const ObjectRef& obj, const 
TargetIdNode::ValueTypeInfo& inf
   }
 }
 
-TVM_DLL void TargetIdNode::ValidateSchema(const Map<String, ObjectRef>& 
config) const {
+void TargetIdNode::ValidateSchema(const Map<String, ObjectRef>& config) const {
+  const String kTargetId = "id";
   for (const auto& kv : config) {
-    auto it = key2vtype_.find(kv.first);
+    const String& name = kv.first;
+    const ObjectRef& obj = kv.second;
+    if (name == kTargetId) {
+      CHECK(obj->IsInstance<StringObj>())
+          << "AttributeError: \"id\" is not a string, but its type is " << 
obj->GetTypeKey();
+      CHECK(Downcast<String>(obj) == this->name)
+          << "AttributeError: \"id\" = " << obj << " is inconsistent with 
TargetId " << this->name;
+      continue;
+    }
+    auto it = key2vtype_.find(name);
     if (it == key2vtype_.end()) {
       std::ostringstream os;
-      os << "AttributeError: Invalid config option, cannot recognize \'" << 
kv.first
-         << "\' candidates are:";
-      bool is_first = true;
+      os << "AttributeError: Invalid config option, cannot recognize \'" << 
name
+         << "\'. Candidates are:";
       for (const auto& kv : key2vtype_) {
-        if (is_first) {
-          is_first = false;
-        } else {
-          os << ',';
-        }
-        os << ' ' << kv.first;
+        os << "\n  " << kv.first;
       }
       LOG(FATAL) << os.str();
       throw;
     }
-    const auto& obj = kv.second;
     const auto& info = it->second;
     try {
       VerifyTypeInfo(obj, info);
     } catch (const tvm::Error& e) {
-      LOG(FATAL) << "AttributeError: Schema validation failed for TargetId " 
<< name
+      LOG(FATAL) << "AttributeError: Schema validation failed for TargetId " 
<< this->name
                  << ", details:\n"
                  << e.what() << "\n"
-                 << "The given config is:\n"
+                 << "The config is:\n"
                  << config;
       throw;
     }
   }
 }
 
+inline String GetId(const Map<String, ObjectRef>& target, const char* name) {
+  const String kTargetId = "id";
+  CHECK(target.count(kTargetId)) << "AttributeError: \"id\" does not exist in 
" << name << "\n"
+                                 << name << " = " << target;
+  const ObjectRef& obj = target[kTargetId];
+  CHECK(obj->IsInstance<StringObj>()) << "AttributeError: \"id\" is not a 
string in " << name
+                                      << ", but its type is " << 
obj->GetTypeKey() << "\n"
+                                      << name << " = " << target;
+  return Downcast<String>(obj);
+}
+
+void TargetValidateSchema(const Map<String, ObjectRef>& config) {
+  try {
+    const String kTargetHost = "target_host";
+    Map<String, ObjectRef> target = config;
+    Map<String, ObjectRef> target_host;
+    String target_id = GetId(target, "target");
+    String target_host_id;
+    if (config.count(kTargetHost)) {
+      target.erase(kTargetHost);
+      target_host = Downcast<Map<String, ObjectRef>>(config[kTargetHost]);

Review comment:
       Seems we are special casing target host here. Is it possible to simply 
set the type to be Target and not do special casing. For example, in the 
composite target case, we are actually looking for `Array<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.

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


Reply via email to