Implement digamma by zou3519 · Pull Request #3955 · pytorch/pytorch

zou3519 · 2017-11-30T20:04:38Z

Implements torch.digamma and tensor.digamma() as per #678

cc @fritzo

Test Plan

New unit tests. cpu unit tests test some edge cases, gpu unit test compares output of cpu to gpu

pytorchbot · 2017-11-30T20:04:38Z

@zou3519, thanks for your PR! We identified @zdevito to be a potential reviewer.

fritzo

Thanks for implementing this!

Sign in to view


+    def test_digamma(self):
+        y = torch.Tensor([-10, 0])
+        x = torch.Tensor([-0.1, 3, 999])


gchanan · 2017-12-01T19:50:14Z

fwiw I'm adding the ability to write efficient pointwise native functions in ATen directly.

Sign in to view


 - name: lgamma(Tensor self)
-  self: not_implemented("lgamma")
+  self: grad * digamma(self)


apaszke

Looks good. We should resolve the licensing issues before merging this.

Sign in to view

+    return THTensor_(digamma_one)(1 - x) - PI / tan(PI * x);
+  }
+
+	// Push x to be >= 10


Sign in to view

+}
+
+/*
+ * Algorithm adapted from Cephes


Sign in to view

        tensor = tensor.unsqueeze(1)
        self.assertEqual(tensor.var(0)[0], 0.03125)

+    def test_digamma(self):


Sign in to view


 - name: lgamma(Tensor self)
-  self: not_implemented("lgamma")
+  self: grad * digamma(self)


fritzo · 2017-12-09T20:02:08Z

FYI this will merge conflict with #3978 where I've implemented a makeshift finite-difference version of THTensor(digamma_one). If #3978 merges before this PR, you can simply delete my makeshift version.

fritzo · 2017-12-15T04:04:49Z

@zou3519 can I help with this at all? Now that you've done the hard part, I'm happy to resolve merge conflicts and add tests. (We're looking forward to using this in Pyro, and we already have a wrapper to use torch.distributions.Gamma).

zou3519 · 2017-12-15T04:06:33Z

@fritzo I haven't reached out to the author about the licensing yet. I'll do that tomorrow and we'll see how that goes :)

fritzo · 2017-12-15T04:07:36Z

I see, thanks for letting me know!

ezyang · 2017-12-21T00:55:20Z

Any update on the license status?

zou3519 · 2017-12-21T01:00:58Z

Sent an email a few days ago to the author, haven't heard back yet

fritzo · 2017-12-28T01:48:53Z

If we still haven't heard from the author, I could throw together a little PR that exposes the makeshift finite-difference implementation of digamma that we're already using internally in a few places. This would at least unblock users of Gamma, Beta, and Dirichlet distributions.

pytorch/aten/src/ATen/native/Distributions.cpp

Line 21 in 15b657a

// TODO Replace this with more accurate digamma().
pytorch/aten/src/TH/generic/THTensorMath.c

Line 3483 in 15b657a

// TODO Replace this with more accurate digamma().
pytorch/torch/distributions/utils.py

Line 6 in 15b657a

# TODO Remove this once torch.digamma is implemented.

apaszke · 2017-12-28T15:30:30Z

I think merging an approximation is good for now

fritzo · 2017-12-28T22:17:30Z

Hmm I looked into it but it appears to be quite complex now that our functions using digamma are spread across C code in aten/src/TH/generic/THTensorMath.c and C++ code in aten/src/ATen/native/Distributions.cpp. @gchanan would it be difficult for you to move dirichlet_grad() to C++ as well so we can implement digamma() in a single place?

gchanan · 2017-12-28T23:23:24Z

@fritzo sure, I'll take a look at.

zou3519 · 2018-01-23T21:35:16Z

Licensing issues resolved, PR rebased. I was told that "You are welcome to modify and distribute the library under BSD license." so I've added the original copyright notices into the code as comments.

fritzo · 2018-01-23T22:06:14Z

That's great news, @zou3519 !

Added test to check digamma float vs double. TestCuda.test_digamma checks the CUDA {float, double} implementation against the CPU {float, double} implementation.

…igamma

wranai · 2018-03-31T12:16:42Z

Any news on this? It seems only the GPU build failed on Windows last November, and that only because it ran out of memory.

fritzo · 2018-03-31T17:45:34Z

@wranai A simpler implementation of digamma and trigamma is already in master. Try torch.digamma

Fixes pytorch#6190. This is a rebase of pytorch#3955 with some tweaks for better performance around poles. The code is ported over from cephes with permission. By itself, the cephes code returns inf for the poles. For better performance around the poles with float32, one intermediate step is always computed with double precision, regardless of dtype. This step does `PI / tan(PI * input)`. This is necessary because small (1e-6) rounding errors for the inputs to tan have strong effects on the output (ie, the derivative of tan is very large at some points).

* More precise digamma Fixes #6190. This is a rebase of #3955 with some tweaks for better performance around poles. The code is ported over from cephes with permission. By itself, the cephes code returns inf for the poles. For better performance around the poles with float32, one intermediate step is always computed with double precision, regardless of dtype. This step does `PI / tan(PI * input)`. This is necessary because small (1e-6) rounding errors for the inputs to tan have strong effects on the output (ie, the derivative of tan is very large at some points). * Replace usages of finite-differences digamma with newly implemented digamma * Better behavior near and at poles * ScalarConvert -> scalar_cast for readability

* More precise digamma Fixes pytorch#6190. This is a rebase of pytorch#3955 with some tweaks for better performance around poles. The code is ported over from cephes with permission. By itself, the cephes code returns inf for the poles. For better performance around the poles with float32, one intermediate step is always computed with double precision, regardless of dtype. This step does `PI / tan(PI * input)`. This is necessary because small (1e-6) rounding errors for the inputs to tan have strong effects on the output (ie, the derivative of tan is very large at some points). * Replace usages of finite-differences digamma with newly implemented digamma * Better behavior near and at poles * ScalarConvert -> scalar_cast for readability

fritzo reviewed Dec 1, 2017

View reviewed changes

Comment thread test/test_torch.py Outdated

def test_digamma(self):

y = torch.Tensor([-10, 0])

x = torch.Tensor([-0.1, 3, 999])

This comment was marked as off-topic.

Sign in to view

fritzo reviewed Dec 1, 2017

View reviewed changes

Comment thread tools/autograd/derivatives.yaml

- name: lgamma(Tensor self)

self: not_implemented("lgamma")

self: grad * digamma(self)

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

apaszke reviewed Dec 2, 2017

View reviewed changes

This was referenced Dec 9, 2017

Implement torch.digamma() probtorch/pytorch#17

Closed

Implement reparameterized gradient for Gamma sampler #3978

Merged

fritzo mentioned this pull request Dec 15, 2017

Wrap torch.distributions.Gamma, Beta pyro-ppl/pyro#630

Merged

This was referenced Dec 17, 2017

Wrap Cephes library for mathematical special functions #3877

Open

Implement differentiable .entropy() methods probtorch/pytorch#36

Closed

Numerical issues in pyro.distributions.util.log_gamma pyro-ppl/pyro#567

Closed

onnxbot-worker-2 mentioned this pull request Dec 18, 2017

[auto] pytorch-pr-3955 onnxbot/onnx-fb-universe#53

Closed

fritzo mentioned this pull request Dec 20, 2017

Implement .entropy() methods for all distributions #4268

Merged

This was referenced Dec 29, 2017

Add low-precision digamma() and polygamma() functions #4399

Merged

Wrap pytorch Uniform and Cauchy distributions pyro-ppl/pyro#657

Merged

zou3519 force-pushed the digamma branch from 3e53576 to fcf1210 Compare January 23, 2018 21:33

zou3519 force-pushed the digamma branch from fcf1210 to ada63bd Compare January 24, 2018 15:12

onnxbot-worker-1 mentioned this pull request Feb 9, 2018

[auto] pytorch-pr-3955 onnxbot/onnx-fb-universe#618

Closed

zou3519 added 3 commits February 14, 2018 11:58

Implement digamma

a552632

Better test coverage for digamma.

f6b83f6

Added test to check digamma float vs double. TestCuda.test_digamma checks the CUDA {float, double} implementation against the CPU {float, double} implementation.

Replace usages of finite-differences digamma with newly implemented d…

2ae3200

…igamma

zou3519 force-pushed the digamma branch from ada63bd to 2ae3200 Compare February 14, 2018 21:05

soumith closed this Mar 31, 2018

zou3519 mentioned this pull request Apr 2, 2018

[PyTorch]: bad digamma float32 accuracy #6190

Closed

zou3519 mentioned this pull request Apr 11, 2018

More precise digamma #6517

Merged

Conversation

zou3519 commented Nov 30, 2017

Test Plan

Uh oh!

pytorchbot commented Nov 30, 2017

Uh oh!

fritzo left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

Uh oh!

gchanan commented Dec 1, 2017

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

apaszke left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

fritzo commented Dec 9, 2017

Uh oh!

fritzo commented Dec 15, 2017

Uh oh!

zou3519 commented Dec 15, 2017

Uh oh!

fritzo commented Dec 15, 2017

Uh oh!

ezyang commented Dec 21, 2017

Uh oh!

zou3519 commented Dec 21, 2017

Uh oh!

fritzo commented Dec 28, 2017

Uh oh!

apaszke commented Dec 28, 2017

Uh oh!

fritzo commented Dec 28, 2017

Uh oh!

gchanan commented Dec 28, 2017

Uh oh!

zou3519 commented Jan 23, 2018

Uh oh!

fritzo commented Jan 23, 2018

Uh oh!

wranai commented Mar 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fritzo commented Mar 31, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

wranai commented Mar 31, 2018 •

edited

Loading