-
Notifications
You must be signed in to change notification settings - Fork 769
replace assertions by highlight in JIT tests to simple regex matches in PyTorch v1.10.0 tests #15073
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
replace assertions by highlight in JIT tests to simple regex matches in PyTorch v1.10.0 tests #15073
Conversation
|
@boegelbot please test @ generoso |
|
@lexming: Request for testing this PR well received on login1 PR test command '
Test results coming soon (I hope)... - notification for comment with ID 1057494985 processed Message to humans: this is just bookkeeping information for me, |
|
Test report by @boegelbot |
|
Looks reasonable to me. Any idea why we haven't hit this with the original PR: does it just happen on certain systems and/or under certain circumstances? Out of personal interest: do you happen to know if this is still in PyTorch-1.11 as well? I've created an EasyConfig for 1.11-rc4 that I'd like to contribute once the final release is out. Just wondering if I should include this patch right away :) Edit: to answer my own question: I looked at |
|
Test report by @casparvl |
|
@casparvl this is a good a question, but I'm afraid I do not have a clear answer. I also wonder how nobody else hit this issue before me. On our side, these failures are systematic, the same tests fail in every build. I have the suspicion that it might be related to using a temp dir in a shared filesystem. But I have no real proof. I'll try to run a couple of tests to shed some more light. |
|
@lexming Any updates on this? To me, the patch seems quite low impact (it's "only" about a test, and my understanding is that it still checks the results). At the same time, I saw the original issue on the PyTorch issue tracker, and saw the devs weren't too keen on making this change themselves - though I couldn't 100% understand their argumentation. |
|
Test report by @boegel |
|
Test report by @boegel |
|
UPDATE: my previous test worked because it got patched with this patch actually. The installation from current Full build log: https://gist.github.com/lexming/72be3627f389cabdd4ebc95361d1f79c So my suspicions about the storage were not correct. I'll provide more info in the issue upstream, but this patch is still mandatory for us. |
|
Test report by @Flamefire |
|
Test report by @Flamefire |
|
Abandoned. Patch will stay available at https://github.com/lexming/easybuild-easyconfigs/tree/vub-hpc/easybuild/easyconfigs/p/PyTorch |
|
Maybe rebase this after #15904 was merged which got the other tests working |
(created using
eb --new-pr)Workaround for issue pytorch/pytorch#72516
Some tests fail despite working as intended because the assertion by highlight fails to properly catch the highlighted part in the error message. Replacing those assertions with a simple regex match solves the issue.