Run tests/unit/model_parallelism in the modal GPU workflow - #8284
Conversation
The modal-torch-latest workflow only selected tests under tests/unit/v1, while every test in tests/unit/model_parallelism is a multi-rank DistributedTest that the CPU workflow skips via its device-count gate, so the directory was never executed in PR CI at all. Add the directory to the modal test scopes and derive the controller's target validation and all-mode check from the same scope list so the selector and the validator cannot drift apart. Signed-off-by: Guokai Ma <guokai.ma@intel.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ac9b67f85
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| WorkflowConfig( | ||
| name="modal-torch-latest", | ||
| test_scopes=("tests/unit/v1", ), | ||
| test_scopes=("tests/unit/v1", "tests/unit/model_parallelism"), |
There was a problem hiding this comment.
This is a non-merge commit, but its message has no Signed-off-by trailer. Please recreate the commit with --signoff so it satisfies the repository's mandatory commit policy.
AGENTS.md reference: AGENTS.md:L8-L8
Useful? React with 👍 / 👎.
|
@delock please move |
|
Closing in favor of the simpler path: moved |
|
@sfc-gh-truwase Done — moved to |
2 participants
What this does
tests/unit/model_parallelismwas never executed in PR CI:DistributedTest(world_size 2/4), andDistributedExec._launch_procs(tests/unit/common.py) skips them because a CPU runner reportsdevice_count() == 1.tests/unit/v1.This came to light in #8241, whose regression test for #8231 (
TestAutoTPMultipleModels) is in that directory and therefore ran nowhere in CI.Changes
ci/tests_fetcher.py: addtests/unit/model_parallelismto the modal workflowtest_scopes.ci/torch_latest.py: derive_TEST_FILE_REand the all-mode selection check from the workflow'stest_scopes(imported fromtests_fetcher) instead of hardcodingtests/unit/v1, so the selector and the validator share one source of truth.ci/test_torch_latest.py: update the self-tests for the two-scope reality (all-mode selection content, a model_parallelism subset target, new invalid cases,count=assertion).Validation
python3 ci/test_tests_fetcher.py— 16/16 passedpython3 ci/test_torch_latest.py— 20/20 passeddeepspeed/module_inject/tp_shard.py,ci/tests_fetcher.py --workflow modal-torch-latestselects all 6model_parallelismtest files (mode=subset, 45 targets), andci/torch_latest.py validate-selection --mode subsetaccepts the list.Notes
pull_request_target), so this only takes effect for other PRs once it lands on master; this PR's own run validates the selection plumbing but will still select v1-only tests for its own diff.tp_size=4tests will still be skipped on the 2-GPU modal sandbox via the existing device-count gate; world_size=2 tests (includingTestAutoTPMultipleModels) will run for real.