Skip to content

Conversation

pmeier
Copy link
Collaborator

@pmeier pmeier commented Mar 6, 2023

Stack from ghstack (oldest at bottom):

Per title. The major ones:

  • Enforce keyword only parameters for _modify_low_high, which takes 7 parameters.
    _modify_low_high(low, high, ranges[0], ranges[1], 0, 10, dtype),

    is just impossible to comprehend without multiple trips back and forth.
  • Improve the error messages by including the offending values in the message

I'll highlight the smaller ones inline.

@pytorch-bot
Copy link

pytorch-bot bot commented Mar 6, 2023

🔗 Helpful Links

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

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

❌ 1 Failures

As of commit eaa1141:

NEW FAILURES - The following jobs have failed:

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

@pmeier pmeier added module: testing Issues related to the torch.testing module (not tests) topic: not user facing topic category labels Mar 6, 2023
Per title. The major ones:

- Enforce keyword only parameters for `_modify_low_high`, which takes 7 parameters.
  https://github.com/pytorch/pytorch/blob/28aa2efd1422e689d3a7dc6b447ca1f31bbad1c3/torch/testing/_creation.py#L147
  is just impossible to comprehend without multiple trips back and forth.
- Improve the error messages by including the offending values in the message

I'll highlight the smaller ones inline.

[ghstack-poisoned]
pmeier added a commit that referenced this pull request Mar 6, 2023
ghstack-source-id: 384473d
Pull Request resolved: #96125
@pmeier pmeier requested a review from mruberry March 6, 2023 20:24
Copy link
Collaborator

@mruberry mruberry left a comment

Choose a reason for hiding this comment

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

Per title. The major ones:

- Enforce keyword only parameters for `_modify_low_high`, which takes 7 parameters.
  https://github.com/pytorch/pytorch/blob/28aa2efd1422e689d3a7dc6b447ca1f31bbad1c3/torch/testing/_creation.py#L147
  is just impossible to comprehend without multiple trips back and forth.
- Improve the error messages by including the offending values in the message

I'll highlight the smaller ones inline.

[ghstack-poisoned]
import torch

# Used by make_tensor for generating complex tensor.
complex_to_corresponding_float_type_map = {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We don't need that since torch.finfo works for complex types as well.

Comment on lines +11 to +15
_INTEGRAL_TYPES = [torch.uint8, torch.int8, torch.int16, torch.int32, torch.int64]
_FLOATING_TYPES = [torch.float16, torch.bfloat16, torch.float32, torch.float64]
_COMPLEX_TYPES = [torch.complex32, torch.complex64, torch.complex128]
_BOOLEAN_OR_INTEGRAL_TYPES = [torch.bool, *_INTEGRAL_TYPES]
_FLOATING_OR_COMPLEX_TYPES = [*_FLOATING_TYPES, *_COMPLEX_TYPES]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Defining them here do avoid doing it over and over again inside make_tensor.

@pmeier pmeier requested a review from mruberry March 8, 2023 20:39
pmeier added 2 commits March 16, 2023 11:54
Per title. The major ones:

- Enforce keyword only parameters for `_modify_low_high`, which takes 7 parameters.
  https://github.com/pytorch/pytorch/blob/28aa2efd1422e689d3a7dc6b447ca1f31bbad1c3/torch/testing/_creation.py#L147
  is just impossible to comprehend without multiple trips back and forth.
- Improve the error messages by including the offending values in the message

I'll highlight the smaller ones inline.

[ghstack-poisoned]
Per title. The major ones:

- Enforce keyword only parameters for `_modify_low_high`, which takes 7 parameters.
  https://github.com/pytorch/pytorch/blob/28aa2efd1422e689d3a7dc6b447ca1f31bbad1c3/torch/testing/_creation.py#L147
  is just impossible to comprehend without multiple trips back and forth.
- Improve the error messages by including the offending values in the message

I'll highlight the smaller ones inline.

[ghstack-poisoned]
Per title. The major ones:

- Enforce keyword only parameters for `_modify_low_high`, which takes 7 parameters.
  https://github.com/pytorch/pytorch/blob/28aa2efd1422e689d3a7dc6b447ca1f31bbad1c3/torch/testing/_creation.py#L147
  is just impossible to comprehend without multiple trips back and forth.
- Improve the error messages by including the offending values in the message

I'll highlight the smaller ones inline.

[ghstack-poisoned]
@pmeier pmeier added the ciflow/trunk Trigger trunk jobs on your pull request label Mar 20, 2023
Copy link
Collaborator

@mruberry mruberry left a comment

Choose a reason for hiding this comment

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

Per title. The major ones:

- Enforce keyword only parameters for `_modify_low_high`, which takes 7 parameters.
  https://github.com/pytorch/pytorch/blob/28aa2efd1422e689d3a7dc6b447ca1f31bbad1c3/torch/testing/_creation.py#L147
  is just impossible to comprehend without multiple trips back and forth.
- Improve the error messages by including the offending values in the message

I'll highlight the smaller ones inline.

[ghstack-poisoned]
Per title. The major ones:

- Enforce keyword only parameters for `_modify_low_high`, which takes 7 parameters.
  https://github.com/pytorch/pytorch/blob/28aa2efd1422e689d3a7dc6b447ca1f31bbad1c3/torch/testing/_creation.py#L147
  is just impossible to comprehend without multiple trips back and forth.
- Improve the error messages by including the offending values in the message

I'll highlight the smaller ones inline.

[ghstack-poisoned]
@pytorchmergebot
Copy link
Collaborator

Rebased gh/pmeier/59/orig onto refs/remotes/origin/master because #96334 was rebased, please pull locally before adding more changes (for example, via ghstack checkout https://github.com/pytorch/pytorch/pull/96125)

pmeier added 2 commits March 22, 2023 13:39
Per title. The major ones:

- Enforce keyword only parameters for `_modify_low_high`, which takes 7 parameters.
  https://github.com/pytorch/pytorch/blob/28aa2efd1422e689d3a7dc6b447ca1f31bbad1c3/torch/testing/_creation.py#L147
  is just impossible to comprehend without multiple trips back and forth.
- Improve the error messages by including the offending values in the message

I'll highlight the smaller ones inline.

[ghstack-poisoned]
Per title. The major ones:

- Enforce keyword only parameters for `_modify_low_high`, which takes 7 parameters.
  https://github.com/pytorch/pytorch/blob/28aa2efd1422e689d3a7dc6b447ca1f31bbad1c3/torch/testing/_creation.py#L147
  is just impossible to comprehend without multiple trips back and forth.
- Improve the error messages by including the offending values in the message

I'll highlight the smaller ones inline.

[ghstack-poisoned]
@pytorchmergebot
Copy link
Collaborator

Rebased gh/pmeier/59/orig onto refs/remotes/origin/viable/strict because #96334 was rebased, please pull locally before adding more changes (for example, via ghstack checkout https://github.com/pytorch/pytorch/pull/96125)

@facebook-github-bot facebook-github-bot deleted the gh/pmeier/53/head branch June 8, 2023 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk Trigger trunk jobs on your pull request Merged module: testing Issues related to the torch.testing module (not tests) open source topic: not user facing topic category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants