-
Notifications
You must be signed in to change notification settings - Fork 177
[CI] Refactor CI #952
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
[CI] Refactor CI #952
Conversation
475f9bb
to
74bcd9c
Compare
f183ad6
to
1d51c1e
Compare
pytest -sv tests/singlecard/test_ilama_lora.py | ||
pytest -sv tests/ops | ||
pytest -sv tests/compile | ||
# AscendScheduler doesn't work, fix it later |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pytest -sv tests/compile | ||
# AscendScheduler doesn't work, fix it later | ||
# pytest -sv tests/singlecard/tets_schedule.py | ||
# guided decoding doesn't work, fix it later |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be fixed by @shen-shanshan in #969
@@ -322,6 +322,7 @@ def test_deterministic_when_seeded( | |||
assert torch.equal(results[j][i], results[0][i]) | |||
|
|||
|
|||
@pytest.mark.skipif(True, reason="Test failed, need fix") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be fixed by @ponix-j
8bad69b
to
13c3d01
Compare
@@ -59,9 +57,9 @@ def test_basic_camem(): | |||
assert torch.allclose(output, torch.ones_like(output) * 3) | |||
|
|||
|
|||
@pytest.mark.skipif(True, reason="test failed, should be fixed later") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be fixed by @Potabk
pytest -sv tests/singlecard/test_offline_inference.py | ||
pytest -sv tests/ops | ||
# AscendScheduler doesn't work, fix it later |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto @zzzzwwjj
pytest -sv tests/ops | ||
# AscendScheduler doesn't work, fix it later | ||
# pytest -sv tests/singlecard/tets_schedule.py | ||
# guided decoding doesn't work, fix it later |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto @shen-shanshan
if __name__ == "__main__": | ||
import pytest | ||
pytest.main([__file__]) | ||
@pytest.mark.skipif(os.getenv("VLLM_USE_V1") == "1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be fixed by @MengqingCao when v1 is ready
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM if CI passed
# spec decode test | ||
VLLM_USE_MODELSCOPE=true pytest -sv tests/long_term/spec_decode/e2e/test_v1_spec_decode.py | ||
VLLM_USE_MODELSCOPE=True pytest -sv tests/long_term/spec_decode/e2e/test_mtp_correctness.py # it needs a clean process | ||
pytest -sv tests/long_term/spec_decode --ignore=tests/long_term/spec_decode/e2e/test_mtp_correctness.py --ignore=tests/long_term/spec_decode/e2e/test_v1_spec_decode.py |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also cc @mengwei805 the spec decode will be triggered by manaully after this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the commit message:
1. decide which test need run, then label it. It can be `long-term-test` or `pd-test` or both.
2. add `ready-for-test` label, then the test will be ran.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i consider it is a wonderful refactor
Signed-off-by: wangxiyuan <wangxiyuan1007@gmail.com>
1. remove some useless test func and file 2. fix format.sh problem 3. enable full test for singlecard and multicard 4. move long term test to long_term folder. For this kind of test, it only runs by labeled and daily test. Include: spec decode、accuracy test There are 4 test modules - `singlecard`: contains the test running on one NPU. It'll be run for each PR and daily test. - `multicard`: contains the test running on multi NPUs. It'll be run for each PR and daily test. - `long_term`: contains the test that cost much time(Now include `spec decode` and `accuracy` test). It'll be run for the PR with `long-term-test` labeled and daily test. - `e2e`: contains the test for doc and pd feature. It'll be run for the PR with `pd-test` labeled and daily test. 1. some test are skipped, they should be fixed and reenabled in the future. 2. pyhccl test for multicard doesn't work at all. It should be enabled as well. 3. ensure long-term-test pass by daily test. Now, `ready` labels is required to start pd test or long term test. And when `long-term-test` or `pd-test` is labeled after another one, the old labeled test will be re-run again. So the labeled test should be ran in the following step: 1. decide which test need run, then label it. `long-term-test` or `pd-test` or both. 2. add `ready-for-test` label, then the test will be ran. Signed-off-by: wangxiyuan <wangxiyuan1007@gmail.com> Signed-off-by: hfadzxy <starmoon_zhang@163.com>
After refactor:
There are 4 test modules
singlecard
: contains the test running on one NPU. It'll be run for each PR and daily test.multicard
: contains the test running on multi NPUs. It'll be run for each PR and daily test.long_term
: contains the test that cost much time(Now includespec decode
andaccuracy
test). It'll be run for the PR withlong-term-test
labeled and daily test.e2e
: contains the test for doc and pd feature. It'll be run for the PR withpd-test
labeled and daily test.Todo:
Know issue
Now,
ready-for-test
labels is required to start pd test or long term test. And whenlong-term-test
orpd-test
is labeled after another one, the old labeled test will be re-run again. So the labeled test should be ran in the following step:long-term-test
orpd-test
or both.ready-for-test
label, then the test will be ran.