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 2020/09/03 17:14:58 UTC

[GitHub] [incubator-tvm] tqchen opened a new pull request #6394: [TESTING] Fix the error when running tests with default targets

tqchen opened a new pull request #6394:
URL: https://github.com/apache/incubator-tvm/pull/6394


   The previous testing.py causes problem of not being able to run the test with the default set of targets.
   
   We might also want to update the code further to memoize the list of targets in the first run
   
   cc @tkonolige @tmoreau89 @jroesch 


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] tqchen edited a comment on pull request #6394: [TESTING] Fix the error when running tests with default targets

Posted by GitBox <gi...@apache.org>.
tqchen edited a comment on pull request #6394:
URL: https://github.com/apache/incubator-tvm/pull/6394#issuecomment-686647893


   `opencl -device=xyz` will fail on my local mac when running tests. 
   
   This happens when I directly run `python tests/python/integration/test_ewise.py`
   
   There is a difference between whether a target is enabled, versus whether a runtime is available.  Most of the runtime enabled code only takes the `target_kind` as input without the additional strings.
   
   For runtime availability testing, we should always use the target kind for now. We are also in the process of upgrading the Target specifications, so we might want to look into that later.
   
   But my take is that most of the cases should be about runtime availability. 
   
   
    


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] tqchen commented on pull request #6394: [TESTING] Fix the error when running tests with default targets

Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #6394:
URL: https://github.com/apache/incubator-tvm/pull/6394#issuecomment-686647893


   `opencl -device=xyz` will fail on my local mac when running tests.
   
   There is a difference between whether a target is enabled, versus whether a runtime is available.  Most of the runtime enabled code only takes the `target_kind` as input without the additional strings.
   
   For runtime availability testing, we should always use the target kind for now. We are also in the process of upgrading the Target specifications, so we might want to look into that later.
   
   But my take is that most of the cases should be about runtime availability. 
   
   
    


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] ZihengJiang merged pull request #6394: [TESTING] Fix the error when running tests with default targets

Posted by GitBox <gi...@apache.org>.
ZihengJiang merged pull request #6394:
URL: https://github.com/apache/incubator-tvm/pull/6394


   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] tkonolige commented on pull request #6394: [TESTING] Fix the error when running tests with default targets

Posted by GitBox <gi...@apache.org>.
tkonolige commented on pull request #6394:
URL: https://github.com/apache/incubator-tvm/pull/6394#issuecomment-686656965


   This test fails: `TVM_TEST_TARGETS="llvm -mtriple=nonexistant" python3 -m pytest tests/python/relay/test_op_level2.py`.
   
   Many of the tests used to use `tvm.runtime.enabled` on the full target string before #6346, which is why I kept the check on the full string. I think the main problem is that checking if something is enabled is not consistent between target/devices.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] tqchen edited a comment on pull request #6394: [TESTING] Fix the error when running tests with default targets

Posted by GitBox <gi...@apache.org>.
tqchen edited a comment on pull request #6394:
URL: https://github.com/apache/incubator-tvm/pull/6394#issuecomment-686664859


   It is a bit confusing to do `runtime.enabled("llvm -mtriple=nonexistant")` test. 
   
   Because `llvm -mtriple=nonexistant` is a target(whether we can generate code for the target), but not necessarily mean that the particular runtime is enabled. e.g. we can generate ARM code on an x86 machine, or opencl code in a machine without opencl devices. But it does not mean that we can run the code on the same env. 
   
   In most of the testcases we are interested right now, we are asking about whether a runtime is enabled(so we can run the generated code). I agree that we might want to improve the logic further. Or modify the testing logic, to use TVM_TESTING_RUNTIME and only include cpu/gpu/opencl/metal/ variations for 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.

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



[GitHub] [incubator-tvm] tkonolige commented on pull request #6394: [TESTING] Fix the error when running tests with default targets

Posted by GitBox <gi...@apache.org>.
tkonolige commented on pull request #6394:
URL: https://github.com/apache/incubator-tvm/pull/6394#issuecomment-686642546


   Which target strings is this failing with?
   
   It seems like we want to test if the full target string is enabled with `tvm.runtime.enabled`. For example `tvm.runtime.enabled("llvm -mtriple=nonexistant")` should not be enabled (but it will be with this patch).


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] tqchen edited a comment on pull request #6394: [TESTING] Fix the error when running tests with default targets

Posted by GitBox <gi...@apache.org>.
tqchen edited a comment on pull request #6394:
URL: https://github.com/apache/incubator-tvm/pull/6394#issuecomment-686647893


   `opencl -device=xyz` will fail on my local mac when running tests. 
   
   This happens when I directly run `python tests/python/integration/test_ewise.py`
   
   There is a difference between whether a target is enabled(can generate code), versus whether a runtime is available(can run).  Most of the runtime enabled code only takes the `target_kind` as input without the additional strings.
   
   For runtime availability testing, we should always use the target kind for now. We are also in the process of upgrading the Target specifications, so we might want to look into that later.
   
   But my take is that most of the cases should be about runtime availability. 
   
   
    


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] tkonolige edited a comment on pull request #6394: [TESTING] Fix the error when running tests with default targets

Posted by GitBox <gi...@apache.org>.
tkonolige edited a comment on pull request #6394:
URL: https://github.com/apache/incubator-tvm/pull/6394#issuecomment-686642546


   Which target strings is this failing with?
   
   It seems like we want to test if the full target string is enabled with `tvm.runtime.enabled`. For example `llvm -mtriple=nonexistant` should not be enabled (but it will be with this patch).


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] tqchen commented on pull request #6394: [TESTING] Fix the error when running tests with default targets

Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #6394:
URL: https://github.com/apache/incubator-tvm/pull/6394#issuecomment-686664859


   It is a bit confusing to do `runtime.enabled("llvm -mtriple=nonexistant")` test. 
   
   Because `llvm -mtriple=nonexistant` is a target(whether we can generate code for the target), but not necessarily mean that the particular runtime is enabled. e.g. we can generate ARM code on an x86 machine, or opencl code in a machine without opencl devices. But it does not mean that we can run the code on the same env. We might want to think about target enabled for the later case.
   
   In most of the testcases we are interested right now, we are asking about whether a runtime is enabled(so we can run the generated code). I agree that we might want to improve the logic further. Or modify the testing logic, to use TVM_TESTING_RUNTIME and only include cpu/gpu/opencl/metal/ variations for 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.

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



[GitHub] [incubator-tvm] tqchen commented on pull request #6394: [TESTING] Fix the error when running tests with default targets

Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #6394:
URL: https://github.com/apache/incubator-tvm/pull/6394#issuecomment-686666259


   That is also why in the original test cases the LLVM arm code generator tests are done in a different way(always enabled because llvm covers these cases), and we don't run the code, but instead just inspect the generated asms


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] tkonolige commented on pull request #6394: [TESTING] Fix the error when running tests with default targets

Posted by GitBox <gi...@apache.org>.
tkonolige commented on pull request #6394:
URL: https://github.com/apache/incubator-tvm/pull/6394#issuecomment-686681682


   I think this change is fine. The real problem seems to be a conflation of targets and devices in a lot of the tests. I think we might also have to improve `tvm.runtime.enabled` and `tvm.context().exist`. It appears that `tvm.context("llvm -mtriple=something").exist` is not accurate.


----------------------------------------------------------------
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:
users@infra.apache.org