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/06/10 21:42:30 UTC

[GitHub] [tvm] tkonolige opened a new pull request, #11676: [FIX,METASCHEDULER] Fix tune_te

tkonolige opened a new pull request, #11676:
URL: https://github.com/apache/tvm/pull/11676

   `tune_te` was broken because it passed a primfunc to `tune_tir`. Now it is wrapped in an IRModule. Also the test is re-enabled.
   
   @junrushao1994 @zxybazh 


-- 
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] zxybazh commented on pull request #11676: [FIX,METASCHEDULER] Fix tune_te

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

   Hi Tristan, thanks for looking into the code for issues. However I think the previous implementation was fine becauase:
   1. `tune_tir` supports tuning of PrimFunc directly. The test could work without the modification.
   2. The test was skipped because it's an integration test not because it's broken. We prefer unittests instead of integration tests in CI.
   
   May I ask if there's any particular reason that we might want to enable this test or put PrimFunc into an IRModule? If you prefer to enable this test I think it would be great if we create a unit test instead for your need : )


-- 
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] junrushao1994 commented on pull request #11676: [FIX,METASCHEDULER] Fix tune_te

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

   Actually I think this line is fine:
   
   ```
   mod = default_config.mod(mod)
   ```
   
   While the tests are disabled only because we think it's more like an integration test that takes time to run, which is not good for the CI


-- 
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] tkonolige commented on pull request #11676: [FIX,METASCHEDULER] Fix tune_te

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

   
   > 1. `tune_tir` supports tuning of PrimFunc directly. The test could work without the modification.
   
   It does not. If you try running without the change to tune.py you will see the test fails.
   
   > 2. The test was skipped because it's an integration test not because it's broken. We prefer unittests instead of integration tests in CI.
   > May I ask if there's any particular reason that we might want to enable this test or put PrimFunc into an IRModule? If you prefer to enable this test I think it would be great if we create a unit test instead for your need : )
   
   Without this test (that you created?) there is no test for `tune_te`. That is how this error got into mainline.
   
   > While the tests are disabled only because we think it's more like an integration test that takes time to run, which is not good for the CI
   
   The test takes 4 seconds on my machine.
   


-- 
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] junrushao1994 commented on pull request #11676: [FIX,METASCHEDULER] Fix tune_te

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

   Yeah we were expecting to have some nightly integration some time on MetaSchedule so that we could catch issues like this, but before this happening, i'm fine if we add extra 4 sec to the CI time :-)


-- 
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] junrushao1994 merged pull request #11676: [FIX,METASCHEDULER] Fix tune_te

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


-- 
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] zxybazh commented on pull request #11676: [FIX,METASCHEDULER] Fix tune_te

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

   I see. It was caused by a recent refactor that removed the line to parse PrimFunc into IRModule, which I haven't rebased on. Thanks for the explanation, it makes a lot more sense now.


-- 
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] junrushao1994 commented on pull request #11676: [FIX,METASCHEDULER] Fix tune_te

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

   BTW, let's call it "MetaSchedule" instead of scheduler given it's a meta way to define schedules as contrary to a specific scheduler


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