Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: leverage Base reduction functions #7

Closed
wants to merge 4 commits into from
Closed

Conversation

simonbyrne
Copy link
Member

This is something I have been meaning to do for a while. Basically the idea is to define a kbn addition operator, and define all the reductions in terms of that.

import Base.TwicePrecision


function plus_kbn(x::T, y::T) where {T}
Copy link
Member

Choose a reason for hiding this comment

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

Should these be restricted to T<:Real? Also do they necessarily have to be the same type? Using + as we do here should take care of any promotion that needs to happen.





Copy link
Member

Choose a reason for hiding this comment

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

This is a lot of blank lines. Could you reduce it to just one, maybe two?

Copy link
Member Author

Choose a reason for hiding this comment

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

No. I need those for caching :trollface:

@ararslan
Copy link
Member

This looks really cool! Is there any noticeable change in performance?

@simonbyrne
Copy link
Member Author

simonbyrne commented Dec 12, 2017

AHHHHH!!!!!

This doesn't work on 0.7 because r_promote was removed (JuliaLang/julia#22825). The recommendation seems to be that you combine the promotion into the mapping function, but that will be pretty inefficient in this case, as it will end up calling plus_kbn(::TwicePrecisionN, ::TwicePrecisionN) instead of plus_kbn(::TwicePrecisionN, ::Float64).

simonbyrne added a commit to JuliaLang/julia that referenced this pull request Dec 12, 2017
Since the demise of `r_promote` in #22825, there is now a type-instability in `mapreduce` if the operator does not give an element of the same type as the input. This arose during my implementation of Kahan summation using a reduction operator, see: JuliaMath/KahanSummation.jl#7

This adds a `mapreduce_single` function which defines what the result should be in these cases.
simonbyrne added a commit to JuliaLang/julia that referenced this pull request Dec 14, 2017
Since the demise of `r_promote` in #22825, there is now a type-instability in `mapreduce` if the operator does not give an element of the same type as the input. This arose during my implementation of Kahan summation using a reduction operator, see: JuliaMath/KahanSummation.jl#7

This adds a `mapreduce_single` function which defines what the result should be in these cases.
simonbyrne added a commit to JuliaLang/julia that referenced this pull request Jan 3, 2018
Since the demise of `r_promote` in #22825, there is now a type-instability in `mapreduce` if the operator does not give an element of the same type as the input. This arose during my implementation of Kahan summation using a reduction operator, see: JuliaMath/KahanSummation.jl#7

This adds a `mapreduce_single` function which defines what the result should be in these cases.
JeffBezanson pushed a commit to JuliaLang/julia that referenced this pull request Jan 4, 2018
Since the demise of `r_promote` in #22825, there is now a type-instability in `mapreduce` if the operator does not give an element of the same type as the input. This arose during my implementation of Kahan summation using a reduction operator, see: JuliaMath/KahanSummation.jl#7

This adds a `mapreduce_single` function which defines what the result should be in these cases.
@mgr327
Copy link

mgr327 commented Jul 4, 2019

Would it be possible to re-activate/implement that pull request?

@JeffreySarnoff
Copy link
Member

JeffreySarnoff commented Jun 7, 2022

We do not care about support for Julia 0.7. If you can get this work as intended for Julia "1.6+", we are open to including it.

@JeffreySarnoff JeffreySarnoff deleted the sb/reduce branch June 7, 2022 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants