Skip to content

Conversation

@malfet
Copy link
Contributor

@malfet malfet commented Jun 19, 2020

Makes errors in cases described in #27815 more obvious

@malfet malfet requested review from ezyang and orionr June 19, 2020 23:34
@ezyang
Copy link
Contributor

ezyang commented Jun 22, 2020

Is this with reference to #27815 ? I thought the plan on record was just to disable this on Windows...

@malfet
Copy link
Contributor Author

malfet commented Jun 22, 2020

@ezyang I can limit the check to just Windows, but can you think of any other 32-bit platform we might want to support?

@malfet malfet force-pushed the malfet/disable-32-bit-support branch from b1ad11f to f3ac5e6 Compare June 22, 2020 18:15
@peterjc123
Copy link
Collaborator

@ezyang I can limit the check to just Windows, but can you think of any other 32-bit platform we might want to support?

So we are rejecting users to build that by themselves, right?

@malfet
Copy link
Contributor Author

malfet commented Jun 22, 2020

So we are rejecting users to build that by themselves, right?
Yes, because 32-bit PyTorch build is currently not supported, but if user tried to build it using 32-bit runtime it fails with a confusing error message.

@malfet malfet changed the title Raise exception when trying to build PyTorch on 32-bit system Raise exception when trying to build PyTorch on 32-bit Windows system Jun 23, 2020
Copy link
Contributor

@ezyang ezyang left a comment

Choose a reason for hiding this comment

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

I might suggest elaborating the error message a bit. The most common reason why bit size is 32 bit is because you have 32 bit Python, but your system might actually be 64 bit, in which case the correct fix is to install 64 bit Python (which is what was the case for the issue people filed.)

@peterjc123
Copy link
Collaborator

peterjc123 commented Jun 23, 2020

I might suggest elaborating the error message a bit. The most common reason why bit size is 32 bit is because you have 32 bit Python, but your system might actually be 64 bit, in which case the correct fix is to install 64 bit Python (which is what was the case for the issue people filed.)

I think it will more useful if we patch the v0.1.2 source so that it will present better message if users try to install a binary package with 32-bit Python on Windows.

@malfet malfet force-pushed the malfet/disable-32-bit-support branch from f3ac5e6 to 89ba564 Compare June 23, 2020 22:22
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@malfet is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@dr-ci
Copy link

dr-ci bot commented Jun 24, 2020

💊 CI failures summary and remediations

As of commit 89ba564 (more details on the Dr. CI page):


  • 1/1 failures introduced in this PR

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

See CircleCI build pytorch_windows_vs2019_py36_cuda10.1_test2 (1/1)

Step: "Test" (full log | diagnosis details | 🔁 rerun)

AssertionError: False is not true
  test_mem_leak (__main__.TestProfiler_cuda) 
Checks that there's no memory leak when using profiler with CUDA ... FAIL (7.350s) 
 
====================================================================== 
FAIL [7.350s]: test_mem_leak (__main__.TestProfiler_cuda) 
Checks that there's no memory leak when using profiler with CUDA 
---------------------------------------------------------------------- 
Traceback (most recent call last): 
  File "test_profiler.py", line 42, in test_mem_leak 
    self.assertTrue(max_diff < 100 * 1024) 
AssertionError: False is not true 
 
---------------------------------------------------------------------- 
Ran 1 test in 7.350s 
 
FAILED (failures=1) 
 
Generating XML reports... 
Generated XML report: test-reports\python-unittest\TEST-TestProfiler_cuda-20200623235249.xml 
Traceback (most recent call last): 
  File "run_test.py", line 727, in <module> 

This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 1 time.

@facebook-github-bot
Copy link
Contributor

@malfet merged this pull request in 883e4c4.

@malfet malfet deleted the malfet/disable-32-bit-support branch June 24, 2020 03:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants