This repository was archived by the owner on Mar 12, 2021. It is now read-only.
Avoid exponential of positive numbers in softplus implementation #518
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Maybe add a test to demonstrate the issue? |
|
```
julia> CuArrays.softplus.([1000.])
1-element Array{Float64,1}:
1000.0
julia> CuArrays.softplus.(CuArray([1000.]))
1-element CuArray{Float64,1,Nothing}:
Inf
```
…On Mon, Dec 2, 2019 at 8:53 AM Rodrigo Vargas ***@***.***> wrote:
```
On Mon, Dec 2, 2019 at 8:39 AM Tim Besard ***@***.***>
wrote:
> Maybe add a test to demonstrate the issue?
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#518?email_source=notifications&email_token=AELWJO4RFAKBULQE7QTOM4DQWS3SXA5CNFSM4JR6IJC2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFSRF2A#issuecomment-560272104>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AELWJO7VCN3Z6O37EDRZ2BDQWS3SXANCNFSM4JR6IJCQ>
> .
>
--
"Io sono giunto a credere che il mondo intero è un enigma, un enigma
innocuo che è reso terribile dalla nostra pessima idea di interpretarlo
come se avesse una verità fondamentale."
-Umberto Eco
--
"Io sono giunto a credere che il mondo intero è un enigma, un enigma
innocuo che è reso terribile dalla nostra pessima idea di interpretarlo
come se avesse una verità fondamentale."
-Umberto Eco
|
|
I mean, to the CuArrays testsuite so that this doesn't regress. |
|
That and a test to make sure this produces the same results as CPU would be good, but otherwise this looks good to me, thanks @vargonis! |
|
Whoa, OK, I'm not really a programmer so apologies for not doing the
complete work (don't even know what "regress" means in this context). Will
try to figure out how tests should be done. That concept is not entirely
new to me, but I have never written a test so please don't count on me
being very efficient with it.
…On Fri, Dec 6, 2019 at 6:29 PM Mike J Innes ***@***.***> wrote:
That and a test to make sure this produces the same results as CPU would
be good, but otherwise this looks good to me, thanks @vargonis
<https://github.com/vargonis>!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#518?email_source=notifications&email_token=AELWJO6D6BLOEBQLAZUY3GDQXKDXPA5CNFSM4JR6IJC2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGEZNJQ#issuecomment-562665126>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AELWJO7Q2FT7LKQ2RZBG6N3QXKDXPANCNFSM4JR6IJCQ>
.
--
"Io sono giunto a credere che il mondo intero è un enigma, un enigma
innocuo che è reso terribile dalla nostra pessima idea di interpretarlo
come se avesse una verità fondamentale."
-Umberto Eco
|
|
No worries, take your time and give us a shout if you're stuck. |
|
OK, I added tests, sorry for the delay. Down here the batchnorm test does
not pass, by the way, but the ones I wrote do. What now? Do I have to make
another pull request, or the original one somehow gets updated?
…On Fri, Dec 6, 2019 at 6:42 PM Mike J Innes ***@***.***> wrote:
No worries, take your time and give us a shout if you're stuck.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#518?email_source=notifications&email_token=AELWJO52K3AXD75XJU6YMHLQXKFIVA5CNFSM4JR6IJC2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGE2R3A#issuecomment-562669804>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AELWJO4BX7MVHMM6HRM2FHLQXKFIVANCNFSM4JR6IJCQ>
.
--
"Io sono giunto a credere che il mondo intero è un enigma, un enigma
innocuo che è reso terribile dalla nostra pessima idea di interpretarlo
come se avesse una verità fondamentale."
-Umberto Eco
|
Are those tests or the failures of it related? If so, I'd fix them here, otherwise create a new PR if you want :-) bors try |
tryBuild succeeded |
|
I can think of no relationship between the modification of sofplus and
batchnorm failures, so I'd say on my side everything is OK.
…On Fri, Dec 20, 2019 at 8:36 AM Tim Besard ***@***.***> wrote:
Down here the batchnorm test does not pass, by the way, but the ones I
wrote do. What now?
Are those tests or the failures of it related? If so, I'd fix them here,
otherwise create a new PR if you want :-)
bors try
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#518?email_source=notifications&email_token=AELWJO5FY3NLZKHBE42TMTTQZRYWHA5CNFSM4JR6IJC2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHMFCDA#issuecomment-567824652>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AELWJO7NSIVXNN6SMC7VJL3QZRYWHANCNFSM4JR6IJCQ>
.
--
"Io sono giunto a credere che il mondo intero è un enigma, un enigma
innocuo che è reso terribile dalla nostra pessima idea di interpretarlo
come se avesse una verità fondamentale."
-Umberto Eco
|
|
Well, I could test to see if batchnorm fails for me on the unmodified
module, sorry for not thinking about that (right now I'm very busy on
something else). But I cannot do that right now (cannot restart my computer
now, and CUDA somehow requires a restart after a sleep :/).
…On Fri, Dec 20, 2019 at 11:49 AM Rodrigo Vargas ***@***.***> wrote:
I can think of no relationship between the modification of sofplus and
batchnorm failures, so I'd say on my side everything is OK.
On Fri, Dec 20, 2019 at 8:36 AM Tim Besard ***@***.***>
wrote:
> Down here the batchnorm test does not pass, by the way, but the ones I
> wrote do. What now?
>
> Are those tests or the failures of it related? If so, I'd fix them here,
> otherwise create a new PR if you want :-)
>
> bors try
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#518?email_source=notifications&email_token=AELWJO5FY3NLZKHBE42TMTTQZRYWHA5CNFSM4JR6IJC2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHMFCDA#issuecomment-567824652>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AELWJO7NSIVXNN6SMC7VJL3QZRYWHANCNFSM4JR6IJCQ>
> .
>
--
"Io sono giunto a credere che il mondo intero è un enigma, un enigma
innocuo che è reso terribile dalla nostra pessima idea di interpretarlo
come se avesse una verità fondamentale."
-Umberto Eco
--
"Io sono giunto a credere che il mondo intero è un enigma, un enigma
innocuo che è reso terribile dalla nostra pessima idea di interpretarlo
come se avesse una verità fondamentale."
-Umberto Eco
|
|
Bump. How does this relate to #539? |
|
It's not fully clear to me, but that could well be meant to solve the same
issue, by using a cutoff instead of resorting to algebraic identities to
avoid exponentiating positive numbers. On the negative side of that
approach, one has precision loss, as mentioned in the request; on the
positive side, possible speed gains.
…On Fri, Jan 17, 2020 at 10:39 AM Tim Besard ***@***.***> wrote:
Bump. How does this relate to #539
<#539>?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#518?email_source=notifications&email_token=AELWJOYKKRU3Z2YZF5BLITTQ6F4GTA5CNFSM4JR6IJC2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJHDISQ#issuecomment-575550538>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AELWJO4DR3LYG7SO7JETVP3Q6F4GTANCNFSM4JR6IJCQ>
.
--
"Io sono giunto a credere che il mondo intero è un enigma, un enigma
innocuo che è reso terribile dalla nostra pessima idea di interpretarlo
come se avesse una verità fondamentale."
-Umberto Eco
|
|
OK thanks, let's go with this PR then. |
bors bot
added a commit
that referenced
this pull request
Jan 17, 2020
518: Avoid exponential of positive numbers in softplus implementation r=maleadt a=vargonis Just use the standard trick to avoid getting `Inf` when broadcasting softplus to GPU arrays containing large positive numbers. Co-authored-by: Rodrigo Vargas <[email protected]>
Build succeeded |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Just use the standard trick to avoid getting
Infwhen broadcasting softplus to GPU arrays containing large positive numbers.