Enable the AMP precision with freezing for CPU nightly test #152298
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/152298
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (2 Unrelated Failures)As of commit a943926 with merge base 190f76f ( FLAKY - The following jobs failed but were likely due to flakiness present on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
@pytorchbot rebase |
|
@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here |
|
Successfully rebased |
57c82f1 to
0c46eb2
Compare
|
@pytorchbot rebase |
|
@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here |
|
Successfully rebased |
6e2734b to
1f901d9
Compare
|
Hi, @desertfire. I enhanced this PR according to your comments. Could you please help review it? |
| name: linux-jammy-cpu-py3.9-gcc11-inductor | ||
| uses: ./.github/workflows/_linux-test.yml | ||
| needs: linux-jammy-cpu-py3_9-gcc11-inductor-build | ||
| if: github.event.schedule == '0 7 * * *' || github.event_name == 'pull_request' |
There was a problem hiding this comment.
This means we will test both freezing ON and OFF every night. Is this really we want here?
There was a problem hiding this comment.
Yes, I remember that you mentioned that we need to keep the default test. Maybe we can just run a few tests with freezing off?
There was a problem hiding this comment.
Did you trigger a dashboard run? Do you have a dashboard link shows how the data is going to be displayed?
There was a problem hiding this comment.
Hi @desertfire. I checked the dashboard page for the performance data related to freezing using the following link . Set the time range to the last 14 days, precision to AMP, and device to CPU (x86).
It appears that the freezing test configuration is already included in the dashboard, but no performance data is being displayed.
There was a problem hiding this comment.
OK, I see two problems here:
- The landing page of the OSS dashboard defaults to bf16 for inference. If a user selects x86 as the inference backend, they will see an empty page thinking the system is down or something.
- If you think freezing should be default, we should avoid testing the non-freezing one to cut the hardware expense. Also the names with freezing on is too long which should be fixed.
UI fixes need to be done in https://github.com/pytorch/test-infra.
There was a problem hiding this comment.
Hi, @desertfire. Now I have removed the non-freezing test, and the test results in the HUD are as follows. Would it make sense to update the CI tests in this PR first, and address the dashboard changes in a follow-up？
There was a problem hiding this comment.
Hi, @huydhn, do you have any insights for the dashboard update? Since we want to update the CPU nightly test using the freezing model.
There was a problem hiding this comment.
Sorry for not seeing your message earlier! It's ok to land this first and update the dashboard later
cc @yangw-dev
| @@ -1,6 +1,9 @@ | |||
| name: inductor-perf-nightly-x86 | |||
|
|
|||
| on: | |||
| pull_request: | |||
| paths: | |||
| - .github/workflows/inductor-perf-test-nightly-x86.yml | |||
There was a problem hiding this comment.
Nit: not really necessary since the file will be updated occasionally, and you can always manually trigger a run when needed.
There was a problem hiding this comment.
I see. I added this code so that the PR can trigger the nightly test. I will remove it later.
|
@pytorchbot rebase |
|
@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here |
|
Successfully rebased |
4a995dc to
3ed5831
Compare
| name: linux-jammy-cpu-py3.9-gcc11-inductor | ||
| uses: ./.github/workflows/_linux-test.yml | ||
| needs: linux-jammy-cpu-py3_9-gcc11-inductor-build | ||
| if: github.event.schedule == '0 7 * * *' | ||
| if: github.event.schedule == '0 7 * * *' || github.event_name == 'pull_request' |
There was a problem hiding this comment.
I assume that you will remove || github.event_name == 'pull_request' part before landing this?
There was a problem hiding this comment.
| if: github.event.schedule == '0 7 * * *' || github.event_name == 'pull_request' | |
| if: github.event.schedule == '0 7 * * *' |
There was a problem hiding this comment.
I assume that you will remove
|| github.event_name == 'pull_request'part before landing this?
Sure. I have removed the PR trigger for this job.
|
@pytorchmergebot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Hi, @desertfire. Since we recommend users to use AMP precision and run with
--freezingfor CPU x86 Inductor inference, we suggest adding the AMP freezing test to the CPU nightly tests.cc @chuanqi129 @zxd1997066