Skip to content

Conversation

@Lunderberg
Copy link
Contributor

As a first step to addressing the Metal codegen errors that required the reversion in #15725, parametrizing the unit tests for allreduce. While these tests are parametrized with @tvm.testing.parametrize_targets("cuda", "metal"), the automatic tvm.testing.requires_metal marker inserted for the metal parametrization will cause them to be skipped if the metal runtime is unavailable, which includes the current CI.

As a first step to addressing the Metal codegen errors that required
the reversion in apache#15725,
parametrizing the unit tests for `allreduce`.  While these tests are
parametrized with `@tvm.testing.parametrize_targets("cuda", "metal")`,
the automatic `tvm.testing.requires_metal` marker inserted for the
metal parametrization will cause them to be skipped if the metal
runtime is unavailable, which includes the current CI.
@Lunderberg
Copy link
Contributor Author

@junrushao @MasterJH5574

Copy link
Contributor

@MasterJH5574 MasterJH5574 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @Lunderberg.

@Lunderberg
Copy link
Contributor Author

Absolutely, @MasterJH5574. I'd like to get the Metal backend to have at least one CI test of lowering/execution, for testing prior to re-applying the CodegenC changes, and to avoid any similar regressions in the future.

Can you verify that the parametrized tests in this PR pass in your environment (pytest command below)? My dev machine runs Linux, and so all the Metal tests are skipped.

TVM_TEST_TARGETS="metal" python3 -mpytest -v tests/python/unittest/test_allreduce.py

@MasterJH5574
Copy link
Contributor

@Lunderberg Yes I can confirm that the test now skips CUDA and passes all Metal tests on mac.

@Hzfengsy Hzfengsy merged commit 64ab31e into apache:main Sep 15, 2023
@Lunderberg Lunderberg deleted the metal_unittest_allreduce branch September 15, 2023 13:31
@Lunderberg
Copy link
Contributor Author

Thank you for the confirmation, and I'm now starting #15756 to enable testing in CI.

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.

3 participants