Skip to content

Add dtype checks in meta dispatch for various ordering ops #159556

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

matthewhagraphcore
Copy link
Collaborator

@matthewhagraphcore matthewhagraphcore commented Jul 31, 2025

This adds data type checks for the unsupported bool and complex types for argmax/min topk, sort, minimum, maximum. As listed here:

https://github.com/pytorch/pytorch/blob/0a99b026d6bd0f67dc2c0a20fe3228ddc4144854/torch/testing/_internal/common_methods_invocations.py#L21076

Currently the ops will fail on CPU or CUDA calculation, rather than at meta dispatch stage as with for example max:

check_unsupported_complex("max()", self);
. This will catch it early.

Copy link

pytorch-bot bot commented Jul 31, 2025

🔗 Helpful Links

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

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

❌ 38 New Failures

As of commit 2155398 with merge base 178515d (image):

NEW FAILURES - The following jobs have failed:

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

Copy link
Contributor

This PR needs a release notes: label

If your changes are user facing and intended to be a part of release notes, please use a label starting with release notes:.

If not, please add the topic: not user facing label.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "topic: not user facing"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@janeyx99
Copy link
Contributor

janeyx99 commented Aug 4, 2025

Test case?

@janeyx99 janeyx99 self-requested a review August 4, 2025 23:12
@janeyx99 janeyx99 added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Aug 4, 2025
@hameerabbasi
Copy link
Collaborator

Thanks for the PR -- I noticed that topk, sort, minimum, maximum all have this behavior. Perhaps it makes sense to extend this PR to those functions as well?

@hameerabbasi
Copy link
Collaborator

hameerabbasi commented Aug 5, 2025

Here are some minimal tests to get one started:

from torch.testing._internal.common_methods_invocations import op_db
from torch.testing._internal.common_device_type import ops

ordered_op_names = {'sort', 'topk', 'lt', 'argmin', 'le', 'ge', 'amax', 'maximum', 'minimum', 'clamp', 'amin', 'gt', 'ceil', 'argmax', 'floor'}
ordered_op_db = tuple(filter(lambda op: op.name in ordered_op_names, op_db))

...
    @ops(ordered_op_db, dtypes=[torch.complex32, torch.complex64, torch.complex128])
    def test_ordered_raises(self, device, dtype, op: OpInfo):
        sample_inputs = op.sample_inputs(device, dtype)

        for sample_input in sample_inputs:
            self.assertRaises(
                NotImplementedError,
                op,
                sample_input.input,
                *sample_input.args,
                **sample_input.kwargs,
            )

@matthewhagraphcore
Copy link
Collaborator Author

I've added the other functions. Sort had a TORCH_CHECK_VALUE instead of a TORCH_CHECK , so to make consistent with the others I've updated that, otherwise it raised a ValueError rather than a RuntimeError,

Not sure what that build error is about?

@matthewhagraphcore matthewhagraphcore changed the title Add dtype checks in meta dispatch for argmin/argmax Add dtype checks in meta dispatch for various ordering ops Aug 8, 2025
@janeyx99
Copy link
Contributor

janeyx99 commented Aug 8, 2025

CI failures are related

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
open source release notes: Meta API release notes category triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants