Skip to content

Conversation

@qqaatw
Copy link
Collaborator

@qqaatw qqaatw commented May 18, 2023

Stack from ghstack (oldest at bottom):

Dispatch the selection function to prevent using is_mps() in Histogram.cpp.

🤖 Generated by Copilot at b329a02

This pull request refactors and implements the logic for inferring the bin edges of histograms from the input tensor for different device types. It introduces a dispatch stub histogram_select_outer_bin_edges_stub and moves the device-specific code to separate files, such as HistogramKernel.cpp and HistogramKernel.mm. This improves the modularity and readability of the histogram functions.

cc @jgong5 @mingfeima @XiaobingSuper @sanchitintel @ashokei @jingxu10

@qqaatw qqaatw mentioned this pull request May 18, 2023
@pytorch-bot
Copy link

pytorch-bot bot commented May 18, 2023

🔗 Helpful Links

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

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 42866e1:
💚 Looks good so far! There are no failures yet. 💚

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

@pytorch-bot pytorch-bot bot added ciflow/mps Run MPS tests (subset of trunk) release notes: mps Release notes category labels May 18, 2023
@github-actions github-actions bot added the module: cpu CPU specific problem (e.g., perf, algorithm) label May 18, 2023
qqaatw added a commit that referenced this pull request May 18, 2023
ghstack-source-id: 00213aa
Pull Request resolved: #101792
@qqaatw qqaatw added the ciflow/trunk Trigger trunk jobs on your pull request label May 18, 2023
@qqaatw qqaatw changed the title [MPS] Dispatch outer bin edges selection [MPS] Dispatch outer bin edges selection function May 18, 2023
Dispatch the selection function to prevent using `is_mps()` in `Histogram.cpp`.

cc jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10

[ghstack-poisoned]
qqaatw added a commit that referenced this pull request May 18, 2023
ghstack-source-id: 6baa972
Pull Request resolved: #101792
@qqaatw qqaatw marked this pull request as ready for review May 18, 2023 10:44
@qqaatw qqaatw requested a review from kulinseth as a code owner May 18, 2023 10:44
@qqaatw qqaatw requested a review from malfet May 18, 2023 12:40
@qqaatw
Copy link
Collaborator Author

qqaatw commented May 23, 2023

@kulinseth @malfet Could you review this stack?

Copy link
Contributor

@malfet malfet left a comment

Choose a reason for hiding this comment

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

Please add test (i.e. if this op is not yet covered by OpInfo, please feel free to add OpInfo test in separate PR on stack)

@qqaatw
Copy link
Collaborator Author

qqaatw commented May 23, 2023

Please add test (i.e. if this op is not yet covered by OpInfo, please feel free to add OpInfo test in separate PR on stack)

We already have opinfo for histc, histogram, and histogramdd, which should cover the change.

Dispatch the selection function to prevent using `is_mps()` in `Histogram.cpp`.

<!--
copilot:summary
-->
### <samp>🤖 Generated by Copilot at b329a02</samp>

This pull request refactors and implements the logic for inferring the bin edges of histograms from the input tensor for different device types. It introduces a dispatch stub `histogram_select_outer_bin_edges_stub` and moves the device-specific code to separate files, such as `HistogramKernel.cpp` and `HistogramKernel.mm`. This improves the modularity and readability of the histogram functions.

cc jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10

[ghstack-poisoned]
Dispatch the selection function to prevent using `is_mps()` in `Histogram.cpp`.

<!--
copilot:summary
-->
### <samp>🤖 Generated by Copilot at b329a02</samp>

This pull request refactors and implements the logic for inferring the bin edges of histograms from the input tensor for different device types. It introduces a dispatch stub `histogram_select_outer_bin_edges_stub` and moves the device-specific code to separate files, such as `HistogramKernel.cpp` and `HistogramKernel.mm`. This improves the modularity and readability of the histogram functions.

cc jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10

[ghstack-poisoned]
qqaatw added a commit that referenced this pull request May 24, 2023
ghstack-source-id: 92c2934
Pull Request resolved: #101792
@qqaatw qqaatw requested a review from malfet May 24, 2023 14:47
@qqaatw
Copy link
Collaborator Author

qqaatw commented May 25, 2023

Gently pinging @malfet

Dispatch the selection function to prevent using `is_mps()` in `Histogram.cpp`.

<!--
copilot:summary
-->
### <samp>🤖 Generated by Copilot at b329a02</samp>

This pull request refactors and implements the logic for inferring the bin edges of histograms from the input tensor for different device types. It introduces a dispatch stub `histogram_select_outer_bin_edges_stub` and moves the device-specific code to separate files, such as `HistogramKernel.cpp` and `HistogramKernel.mm`. This improves the modularity and readability of the histogram functions.

cc jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10

[ghstack-poisoned]
qqaatw added a commit that referenced this pull request May 30, 2023
ghstack-source-id: 38e34fc
Pull Request resolved: #101792
@qqaatw
Copy link
Collaborator Author

qqaatw commented May 30, 2023

@malfet can you please take a look? the failing tests are from broken trunk I think.

@qqaatw
Copy link
Collaborator Author

qqaatw commented Jun 5, 2023

Hey @malfet, can you please review?

@qqaatw
Copy link
Collaborator Author

qqaatw commented Jun 26, 2023

@pytorchbot rebase

@pytorchmergebot
Copy link
Collaborator

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

Dispatch the selection function to prevent using `is_mps()` in `Histogram.cpp`.

<!--
copilot:summary
-->
### <samp>🤖 Generated by Copilot at b329a02</samp>

This pull request refactors and implements the logic for inferring the bin edges of histograms from the input tensor for different device types. It introduces a dispatch stub `histogram_select_outer_bin_edges_stub` and moves the device-specific code to separate files, such as `HistogramKernel.cpp` and `HistogramKernel.mm`. This improves the modularity and readability of the histogram functions.

cc jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10

[ghstack-poisoned]
@pytorchmergebot
Copy link
Collaborator

Successfully rebased gh/qqaatw/33/orig onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via ghstack checkout https://github.com/pytorch/pytorch/pull/101792)

pytorchmergebot pushed a commit that referenced this pull request Jun 26, 2023
ghstack-source-id: 7e8e816
Pull Request resolved: #101792
@qqaatw
Copy link
Collaborator Author

qqaatw commented Jun 26, 2023

Ping @malfet for review, thanks!

@qqaatw
Copy link
Collaborator Author

qqaatw commented Jun 27, 2023

@pytorchbot rebase

@pytorchmergebot
Copy link
Collaborator

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

Dispatch the selection function to prevent using `is_mps()` in `Histogram.cpp`.

<!--
copilot:summary
-->
### <samp>🤖 Generated by Copilot at b329a02</samp>

This pull request refactors and implements the logic for inferring the bin edges of histograms from the input tensor for different device types. It introduces a dispatch stub `histogram_select_outer_bin_edges_stub` and moves the device-specific code to separate files, such as `HistogramKernel.cpp` and `HistogramKernel.mm`. This improves the modularity and readability of the histogram functions.

cc jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10

[ghstack-poisoned]
@pytorchmergebot
Copy link
Collaborator

Successfully rebased gh/qqaatw/33/orig onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via ghstack checkout https://github.com/pytorch/pytorch/pull/101792)

pytorchmergebot pushed a commit that referenced this pull request Jun 27, 2023
ghstack-source-id: 74af5dd
Pull Request resolved: #101792
Copy link
Collaborator

@albanD albanD left a comment

Choose a reason for hiding this comment

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

Thanks for the cleanup!

@qqaatw
Copy link
Collaborator Author

qqaatw commented Jun 27, 2023

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

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 facebook-github-bot deleted the gh/qqaatw/33/head branch July 1, 2023 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/mps Run MPS tests (subset of trunk) ciflow/trunk Trigger trunk jobs on your pull request Merged module: cpu CPU specific problem (e.g., perf, algorithm) open source release notes: mps Release notes category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants