Improved log1p implementation for complex inputs #107100
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/107100
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit ec662a9 with merge base 10ce16b ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
There was a problem hiding this comment.
Since this function is also in c10_complex_math, you don't need to specify the namespace, but fair enough.
Thank you for the quick fix! Feel free to merge the PR yourself via @pytorchbot
|
@pytorchbot merge |
Merge startedYour 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 |
Merge failedReason: 4 mandatory check(s) failed. The first few are:
Dig deeper by viewing the failures on hud |
|
So, as discussed in numpy/numpy#22611 (comment), the implementation in mac seems to be failing. The relative error is small (6e-6) but higher than the tolerance of 1e-6. |
|
And could you also leave a comment as to why we're doing that. |
|
I added a fallback to the old version in case |
|
Great, thank you for the contribution! |
|
@pytorchbot merge |
Merge startedYour 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 |
Merge failedReason: 2 mandatory check(s) failed. The first few are:
Dig deeper by viewing the failures on hud |
|
The failures seem real (although CI is not particularly helpful). Could you look into those? |
|
There seems to be a problem with building |
|
So far, I've not been able to reproduce the build errors on my local setup. I've checked the failing |
|
It could be. I remember that |
c10/util/complex_math.h
Outdated
| } else if (u - T(1) == z) { | ||
| return log(u); | ||
| } else { | ||
| return log(u) * (z / (u - T(1))); |
There was a problem hiding this comment.
Sadly these complex functions are far more costly than the real counterparts and it looks like the build is running out of memory when compiling them. A shot in the dark, there might be a compile time improvement by only calling log(u) once:
auto log_u = log(u);
if (u - T(1) != z) {
return log_u * (z / (u - T(1)));
}
return log_u;
|
All those failures seem unrelated. Can you rebase off |
|
@pytorchbot rebase |
|
@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here |
…espace specifier for call to c10_complex_math::log.
|
Successfully rebased |
pytorchmergebot
force-pushed
the
improved-complex-log1p
branch
from
22e9959 to
ec662a9
Compare
|
Há! That did it! nvcc is sometimes a bit thick... |
|
@pytorchbot merge -i |
Merge startedYour change will be merged while ignoring the following 1 checks: trunk / win-vs2019-cpu-py3 / test (default, 3, 3, windows.4xlarge.nonephemeral) Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
This PR improves the implementation of `torch.log1p` for complex inputs as mentioned in issue pytorch#107022. The new implementation is based on the insights provided in numpy/numpy#22611 (comment). Pull Request resolved: pytorch#107100 Approved by: https://github.com/lezcano
5 participants
This PR improves the implementation of
torch.log1pfor complex inputs as mentioned in issue #107022. The new implementation is based on the insights provided in numpy/numpy#22611 (comment).cc @lezcano