Default TreadPool size to number of physical cores

[malfet]

TODO: Some benchmarks

[pytorch-bot]

🔗 Helpful Links

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

• 📄 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.

❌ 2 New Failures

As of commit be2399a with merge base bbe68a1 ():

NEW FAILURES - The following jobs have failed:

• pull / linux-focal-cuda12.4-py3.10-gcc9 / build (gh)
  /var/lib/jenkins/workspace/aten/src/ATen/cuda/CUDASparseDescriptors.h:119:68: error: ‘cusparseStatus_t cusparseCreateBsrsm2Info(bsrsm2Info**)’ is deprecated: The routine will be removed in the next major release [-Werror=deprecated-declarations]
• pull / linux-focal-cuda12.4-py3.10-gcc9-sm86 / build (gh)
  /var/lib/jenkins/workspace/aten/src/ATen/cuda/CUDASparseDescriptors.h:119:68: error: ‘cusparseStatus_t cusparseCreateBsrsm2Info(bsrsm2Info**)’ is deprecated: The routine will be removed in the next major release [-Werror=deprecated-declarations]

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

[janeyx99]

I do believe this change is for the better but I'm not an expert and so I cannot ascertain that is always better. @malfet what's the plan on the benchmarks haha

[facebook-github-bot]

@janeyx99 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[Skylion007]

Will this work properly for partial CPU allocations on SLURM clusters?

[msaroufim]

This might be a noop concern but was also curious how a change like this affects the performance of distributed jobs

[malfet]

Will this work properly for partial CPU allocations on SLURM clusters?

I can only hope for it :)

[Skylion007]

I'm just worried becaues there might be say 16 logical cores, and only 4 are available to the job meaning that the cores become over-subscribed.

[janeyx99]

@Skylion007 that is the exact type of problem we're attempting to fix, as processors usually means threads and cores means actual cores (what we're thinking). @malfet's PR will be strictly an improvement on that front as written compared to before the change.

[gajjanag]

LGTM - tested both a hyperthreaded multicore and non hyperthreaded multicore. It gives the correct thread count now.

[sanchitintel]

tested both a hyperthreaded multicore and non hyperthreaded multicore. It gives the correct thread count now.

Hi @gajjanag, can you please clarify if you meant you were getting incorrect counts earlier with torch.get_num_threads?
For instance, even with HyperThreading enabled on an Intel machine, I only see physical core count with torch.get_num_threads at my end (without this patch).

Thanks!

[gajjanag]

tested both a hyperthreaded multicore and non hyperthreaded multicore. It gives the correct thread count now.

Hi @gajjanag, can you please clarify if you meant you were getting incorrect counts earlier with torch.get_num_threads? For instance, even with HyperThreading enabled on an Intel machine, I only see physical core count with torch.get_num_threads at my end (without this patch).

Thanks!

Yes, I was getting incorrect counts before this patch (eg a 2 socket 56 core each Intel was giving 224 thread count, but it now gives the correct 112)

[sanchitintel]

Thanks for confirming, @gajjanag!

That hasn't been my experience, with HyperThreading enabled (without this patch) -

[malfet]

Thanks for confirming, @gajjanag!

That hasn't been my experience, with HyperThreading enabled (without this patch) -

We need to chat about this. Perhaps cpuinfo does not correctly work on your system, but it should have returned number of logical cores rather than physical ones.

[malfet]

@pytorchbot rebase

[pytorchmergebot]

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

[pytorchmergebot]

Successfully rebased malfet-patch-29 onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout malfet-patch-29 && git pull --rebase)