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



##########
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:
       I see your point. However, `target_host` is special cased due to the 
following reason
   1) `target_host` should not be nested: there shouldn't be a target_host 
inside another target_host;
   2) Type `target` may need further refactor to be handled properly in the 
validation




----------------------------------------------------------------
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