KEMBAR78
[CPU] Expand `torch.special.i1` to Half and BF16 by malfet · Pull Request #137899 · pytorch/pytorch · GitHub
Skip to content

Conversation

@malfet
Copy link
Contributor

@malfet malfet commented Oct 14, 2024

To match behavior of torch.special.i0

Noticed while looking at the failures in #137849

Also, add explicit high-precision template specialization for calc_i0 and calc_i1 for BFloat16 and Half

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

To match behavior for torch.special.i0 

Noticed while looking at the failures in #137849
@pytorch-bot
Copy link

pytorch-bot bot commented Oct 14, 2024

🔗 Helpful Links

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

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

❌ 1 New Failure

As of commit 030ed3a with merge base f8a5b71 (image):

NEW FAILURE - The following job has failed:

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

@pytorch-bot pytorch-bot bot added the module: cpu CPU specific problem (e.g., perf, algorithm) label Oct 14, 2024
@malfet malfet requested a review from Skylion007 October 14, 2024 16:06
}

// Upcast bfloat16/half input to float for numerical accuracy purposes
inline c10::BFloat16 calc_i1(c10::BFloat16 a) { return calc_i1(static_cast<float>(a)); }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why aren't these just template instatiations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because

inline typename std::enable_if<std::is_floating_point<T>::value, T>::type
and neither Half nor BF16 are considering floating point types. Which reminds me of another simple BE task (enable_if_t :P )

// Upcast bfloat16 input to float for numerical accuracy purposes
// Upcast bfloat16/half input to float for numerical accuracy purposes
inline c10::BFloat16 calc_i0(c10::BFloat16 a) { return calc_i0(static_cast<float>(a)); }
inline c10::Half calc_i0(c10::Half a) { return calc_i0(static_cast<float>(a)); }
Copy link
Collaborator

@Skylion007 Skylion007 Oct 14, 2024

Choose a reason for hiding this comment

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

Same concern

@Skylion007
Copy link
Collaborator

Skylion007 commented Oct 14, 2024

@malfet Need to update optests like:

@malfet malfet added release notes: python_frontend python frontend release notes category topic: improvements topic category labels Oct 14, 2024
@malfet
Copy link
Contributor Author

malfet commented Oct 14, 2024

@malfet Need to update optests like:

I sometimes deliberately delay those to see that our OpInfo is comprehensive enough to fail with unexpected successes...
[Edit] I.e. test_dtypes_special_i1e_cpu should fail, but question is how long will it take to get there :)
[Edit2] And I find out that target-determination determines it's important not to know about the error

@malfet malfet added the ciflow/trunk Trigger trunk jobs on your pull request label Oct 14, 2024
@malfet malfet added the ci-no-td Do not run TD on this PR label Oct 14, 2024
@malfet
Copy link
Contributor Author

malfet commented Oct 14, 2024

@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

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

Dig deeper by viewing the failures on hud

Details for Dev Infra team Raised by workflow job

Failing merge rule: Core Maintainers

@malfet malfet requested a review from mruberry as a code owner October 14, 2024 23:23
@malfet
Copy link
Contributor Author

malfet commented Oct 15, 2024

@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

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

Dig deeper by viewing the failures on hud

Details for Dev Infra team Raised by workflow job

Failing merge rule: Core Maintainers

@malfet malfet requested review from eqy and syed-ahmed as code owners October 15, 2024 03:41
@malfet
Copy link
Contributor Author

malfet commented Oct 15, 2024

@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

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

Dig deeper by viewing the failures on hud

Details for Dev Infra team Raised by workflow job

Failing merge rule: Core Maintainers

@malfet
Copy link
Contributor Author

malfet commented Oct 15, 2024

@pytorchbot merge -i "cpp_threads failure is clearly unrelated"

@pytorch-bot
Copy link

pytorch-bot bot commented Oct 15, 2024

❌ 🤖 pytorchbot command failed:

@pytorchbot: error: unrecognized arguments: cpp_threads failure is clearly unrelated

usage: @pytorchbot [-h] {merge,revert,rebase,label,drci,cherry-pick,close} ...

Try @pytorchbot --help for more info.

@Skylion007
Copy link
Collaborator

@pytorchbot merge -i

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged while ignoring the following 1 checks: pull / linux-jammy-py3.10-clang15-asan / test (default, 6, 6, lf.linux.4xlarge)

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

@github-actions github-actions bot deleted the malfet-patch-2 branch November 15, 2024 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-no-td Do not run TD on this PR ciflow/trunk Trigger trunk jobs on your pull request Merged module: cpu CPU specific problem (e.g., perf, algorithm) release notes: python_frontend python frontend release notes category topic: improvements topic category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants