-
Notifications
You must be signed in to change notification settings - Fork 26.3k
Restore full Windows tests #17102
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
Restore full Windows tests #17102
Conversation
|
EDIT: I don't think JOB_BASE_NAME is being populated anywhere, but now that leads the test to exist b/c we predicate on -z $JOB_BASE_NAME. I think this can by changing https://github.com/pytorch/ossci-job-dsl/blob/master/src/jobs/pytorch.groovy#L1070 to "${buildEnvironment}-test${suffix}" |
|
I think that In the console log of the Similarly, in the console log of the |
|
Can you tell where JOB_BASE_NAME is being populated from? It shouldn't exist anymore. All info should be encoded into BUILD_ENVIRONMENT. |
|
@kostmo Would you please skip |
|
Any updates on this PR? I believe we should fix this ASAP. |
Skip DataLoaderTest.ChunkDataSetGetBatch due to possible deadlock Closes #17101
test/cpp/api/dataloader.cpp
Outdated
| ASSERT_TRUE(batch->target[0].allclose(torch::zeros(kBatchSize - 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.
There should be a comment saying why this is commented out, and an issue tracking when we can uncomment it again.
But IIUC, you only wanted to disable this on Windows, right? Then it should be macro'ed.
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.
Are there pre-existing macros for this, or did you just mean to surround with #ifdef?
ezyang
left a comment
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.
with comments
|
Looks like disabling that test was not enough to fix it: |
|
@pytorchbot retest this please |
|
@kostmo Looks like |
|
Opened a new issue for the deadlock: #17609. |
|
@pytorchbot rebase this please |
|
@pytorchbot rebase this please |
|
@pytorchbot merge this please |
facebook-github-bot
left a comment
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.
@ezyang is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
* upstream/master: (87 commits) Make Variable::set_data non-const; cosmetic fixes. remove warning for upsample code (pytorch#17921) Optimize TileOp (pytorch#17290) Optimize channel_stats_op (pytorch#16243) enable shape inference for elementwise operators (pytorch#17885) Remove remaining test jit expects redux (pytorch#17924) Handle Scalars Better (pytorch#17875) Fixed a formatting issue in doc comments (pytorch#17505) Add nbytes, itemsize, element_size to at::Tensor. (pytorch#17810) Fix lint in test_distributions.py Fix lint in test_jit.py Fix lint errors in test_autograd Added a few extra python bindings to help with walking the IR graph from Python (pytorch#17822) kthvalue consistency with sort in the presence of NaN (pytorch#17824) Fix minor grammatical mistakes in torch/nn/modules/loss.py (pytorch#17892) Remove (almost all) TensorOptions from native_functions.yaml (pytorch#17385) Restore full Windows tests (pytorch#17102) Prevent VS2017 from emitting ambiguous symbol errors (second time) Fix windows test hang (pytorch#17778) torch.btrifact for tensors with greater than 3 dimensions (pytorch#14964) ...
closes #17101