[GitHub] [tvm] sunggg commented on a diff in pull request #12626: [MetaSchedule] Introduce `ScheduleFnDatabase`

[GitBox <gi...@apache.org>]

sunggg commented on code in PR #12626:
URL: https://github.com/apache/tvm/pull/12626#discussion_r957428726


##########
tests/python/unittest/test_link_params.py:
##########
@@ -407,21 +406,21 @@ def schedule_dense(sch):
     target = "llvm"
     params = {"weight": weight_np}
 
-    def schedule_fn(task, sch):
-        if "nn_dense" in task.task_name:
+    def schedule_fn(sch):

Review Comment:
   I'm okay with this for now and agree that this delivers the functionality we need. But like we discussed offline, I have some concerns about this approach and hope to have some discussion in the future PRs.
   - If developers start writing more target-specific, layout-specific manual schedules, we may end up facing complicated if-statement logics that might be hard to maintain.
   - Conditional statement are based on the task name, so when task name is different from what we expect, it wouldn't work. When task naming rule changes, developers might need to update this accordingly, which is not intuitive imo. 
   - I agree with @MasterJH5574, I'm not sure if this is intuitive interface for developers. You would need to modify the schedule and return `True`. Wonder if we can skip manually returning true by automatically checking if the schedule has changed. 
   



##########
src/meta_schedule/database/database.cc:
##########
@@ -156,7 +156,8 @@ TuningRecord TuningRecord::FromJSON(const ObjectRef& json_obj, const Workload& w
 
 /******** Database ********/
 
-Optional<TuningRecord> DatabaseNode::QueryTuningRecord(IRModule mod, Target target) {
+Optional<TuningRecord> DatabaseNode::QueryTuningRecord(const IRModule& mod, const Target& target,
+                                                       const String& workload_name) {

Review Comment:
   Why do we pass `workload_name`? The these three functions do not seem to use this: `QueryTuningRecord`, `QuerySchedule`, `QueryIRModule`. 



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