Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TESTING] Fix the error when running tests with default targets #6394

Merged
merged 1 commit into from Sep 8, 2020

Conversation

tqchen
Copy link
Member

@tqchen tqchen commented Sep 3, 2020

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

@tkonolige
Copy link
Contributor

tkonolige commented Sep 3, 2020

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

@tqchen
Copy link
Member Author

tqchen commented Sep 3, 2020

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.

@tkonolige
Copy link
Contributor

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.

@tqchen
Copy link
Member Author

tqchen commented Sep 3, 2020

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.

@tqchen
Copy link
Member Author

tqchen commented Sep 3, 2020

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

@tkonolige
Copy link
Contributor

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.

@ZihengJiang ZihengJiang merged commit d08eb9a into apache:master Sep 8, 2020
@tqchen tqchen deleted the testing branch February 26, 2023 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants