[optests] Changed failures_dict format to json; automatic update of failures_dict by zou3519 · Pull Request #109110 · pytorch/pytorch

zou3519 · 2023-09-12T13:29:47Z

We changed the failures_dict format from .py to json and added a way to
automatically update the failures dict (the user can set
PYTORCH_OPCHECK_ACCEPT=1 to do so), assuming the tests don't crash in the
process.

Some details:

We introduced a FailuresDict class that handles save/load and from which one
can query a test status ("xfail", "skip", etc).
PYTORCH_OPCHECK_ACCEPT=1 does not override everything. In particular: it
doesn't try to update the failures dict for a test marked as "skip", but it
will update it for tests marked as "xfail" or "success".
PYTORCH_OPCHECK_ACCEPT=1 also does not override the "comment" field, unless
it is flipping an "xfail" into "success".
I'll update the gdoc linked in the comments with how to actually use
PYTORCH_OPCHECK_ACCEPT=1 internally (it's not trivial).

Note that this isn't multithreading-safe, the current recommendation is to run
the tests sequentially if the user wants to use PYTORCH_OPCHECK_ACCEPT=1.

Differential Revision: D49167181

pytorch-bot · 2023-09-12T13:29:50Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/109110

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

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 231ae83 with merge base a46df6e ():

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

lintrunner / linux-job (gh)

UNSTABLE - The following job failed but was likely due to flakiness present on trunk and has been marked as unstable:

linux-focal-cuda12.1-py3.10-gcc9-sm86 / test (default, 1, 5, linux.g5.4xlarge.nvidia.gpu, unstable) (gh)

This comment was automatically generated by Dr. CI and updates every 15 minutes.

facebook-github-bot · 2023-09-12T13:29:58Z

This pull request was exported from Phabricator. Differential Revision: D49167181

facebook-github-bot · 2023-09-12T15:34:35Z

This pull request was exported from Phabricator. Differential Revision: D49167181

facebook-github-bot · 2023-09-12T15:35:53Z

This pull request was exported from Phabricator. Differential Revision: D49167181

facebook-github-bot · 2023-09-12T15:57:16Z

This pull request was exported from Phabricator. Differential Revision: D49167181

facebook-github-bot · 2023-09-12T15:57:28Z

This pull request was exported from Phabricator. Differential Revision: D49167181

facebook-github-bot · 2023-09-12T19:32:59Z

This pull request was exported from Phabricator. Differential Revision: D49167181

facebook-github-bot · 2023-09-12T20:33:23Z

This pull request was exported from Phabricator. Differential Revision: D49167181

ezyang · 2023-09-13T00:53:38Z

test/minioptest_failures_dict.json

would be nice to do the trailing newline correctly

ezyang · 2023-09-13T00:54:07Z

test/test_custom_ops.py

what's this helper function 🤔

Something about internal fbcode having different codepaths to the resource :/

ezyang · 2023-09-13T00:54:45Z

torch/testing/_internal/optests/generate_tests.py

type annotations would be nice

sgtm (will probably save for a follow-up, I need to type the entire file, not just these changes)

ezyang · 2023-09-13T00:57:45Z

torch/testing/_internal/optests/generate_tests.py

interesting...

I guess this feels less bad when you realize this is "expect success", as opposed to "yolo it succeeded"

My mental model is that we're talking about the status of the test:

"skip" means we should skip the test

"xfail" means we expect the test to fail

"success" means we expect the test to succeed.

I could rename this "xsuccess" if we want for consistency (do you have other thoughts?) Though I'll probably land this first to avoid merge conflicts (changing "success" to something else isn't difficult, because this doesn't actually show up in the failure dict unless the user manually puts it in)

ezyang · 2023-09-13T01:18:04Z

torch/testing/_internal/optests/generate_tests.py

ezyang · 2023-09-13T01:19:46Z

properly reviewed now 👍

zou3519 · 2023-09-13T01:50:51Z

Thanks Ed. I need to rebase this anyways because the internal tests are complaining, so, will incorporate your comments

…ailures_dict (pytorch#109110) Summary: We changed the failures_dict format from .py to json and added a way to automatically update the failures dict (the user can set PYTORCH_OPCHECK_ACCEPT=1 to do so), assuming the tests don't crash in the process. Some details: - We introduced a FailuresDict class that handles save/load and from which one can query a test status ("xfail", "skip", etc). - PYTORCH_OPCHECK_ACCEPT=1 does not override everything. In particular: it doesn't try to update the failures dict for a test marked as "skip", but it will update it for tests marked as "xfail" or "success". - PYTORCH_OPCHECK_ACCEPT=1 also does not override the "comment" field, unless it is flipping an "xfail" into "success". - I'll update the gdoc linked in the comments with how to actually use `PYTORCH_OPCHECK_ACCEPT=1` internally (it's not trivial). Note that this isn't multithreading-safe, the current recommendation is to run the tests sequentially if the user wants to use PYTORCH_OPCHECK_ACCEPT=1. bypass-github-export-checks Test Plan: Existing tests Reviewed By: ezyang Differential Revision: D49167181

facebook-github-bot · 2023-09-13T01:57:19Z

This pull request was exported from Phabricator. Differential Revision: D49167181

facebook-github-bot · 2023-09-13T13:21:33Z

@pytorchbot merge -f 'Landed internally'

(Initiating merge automatically since Phabricator Diff has merged, using force because this PR might not pass merge_rules.json but landed internally)

pytorchmergebot · 2023-09-13T13:24:10Z

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use -f as last resort and instead consider -i/--ignore-current to continue the merge ignoring current failures. This will allow currently pending tests to finish and report signal before the merge.

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

facebook-github-bot added the fb-exported label Sep 12, 2023

zou3519 force-pushed the export-D49167181 branch from 18e815f to f8bbf7a Compare September 12, 2023 15:34

zou3519 force-pushed the export-D49167181 branch from f8bbf7a to 113b57b Compare September 12, 2023 15:35

zou3519 added the topic: not user facing topic category label Sep 12, 2023

zou3519 requested review from bdhirsh, ezyang and soulitzer September 12, 2023 15:42

ezyang approved these changes Sep 12, 2023

View reviewed changes

zou3519 force-pushed the export-D49167181 branch from 113b57b to ca76918 Compare September 12, 2023 15:57

zou3519 force-pushed the export-D49167181 branch from ca76918 to 9694443 Compare September 12, 2023 15:57

zou3519 force-pushed the export-D49167181 branch from 9694443 to 0595406 Compare September 12, 2023 17:03

zou3519 force-pushed the export-D49167181 branch from 0595406 to 9c0a3d6 Compare September 12, 2023 20:33

ezyang reviewed Sep 13, 2023

View reviewed changes

torch/testing/_internal/optests/generate_tests.py Outdated

Copy link
Copy Markdown

Contributor

ezyang Sep 13, 2023

Choose a reason for hiding this comment

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

lol

zou3519 reacted with laugh emoji

zou3519 force-pushed the export-D49167181 branch from 9c0a3d6 to 231ae83 Compare September 13, 2023 01:57

pytorchmergebot added the merging label Sep 13, 2023

pytorchmergebot added Merged and removed merging labels Sep 13, 2023

pytorchmergebot closed this in bfa8429 Sep 13, 2023

zou3519 mentioned this pull request Sep 14, 2023

Add opcheck testing for nms pytorch/vision#7961

Merged

Conversation

zou3519 commented Sep 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Sep 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/109110

✅ You can merge normally! (2 Unrelated Failures)

Uh oh!

facebook-github-bot commented Sep 12, 2023

Uh oh!

facebook-github-bot commented Sep 12, 2023

Uh oh!

facebook-github-bot commented Sep 12, 2023

Uh oh!

facebook-github-bot commented Sep 12, 2023

Uh oh!

facebook-github-bot commented Sep 12, 2023

Uh oh!

facebook-github-bot commented Sep 12, 2023

Uh oh!

facebook-github-bot commented Sep 12, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zou3519 Sep 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zou3519 Sep 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ezyang commented Sep 13, 2023

Uh oh!

zou3519 commented Sep 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Sep 13, 2023

Uh oh!

facebook-github-bot commented Sep 13, 2023

Uh oh!

pytorchmergebot commented Sep 13, 2023

Merge started

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

zou3519 commented Sep 12, 2023 •

edited

Loading

pytorch-bot bot commented Sep 12, 2023 •

edited

Loading

zou3519 Sep 13, 2023 •

edited

Loading

zou3519 Sep 13, 2023 •

edited

Loading

zou3519 commented Sep 13, 2023 •

edited

Loading