Skip to content

sum_kbn(fn, A) (issue 3, in part) #6

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.

Sign up for GitHub

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

Closed
wants to merge 15 commits into from
Closed

sum_kbn(fn, A) (issue 3, in part) #6

Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
ef5aef4
vec -> avec (vec is a function)
JeffreySarnoff Dec 11, 2017
4876eb7
https://github.com/JuliaMath/KahanSummation.jl/issues/3 (in part)
JeffreySarnoff Dec 11, 2017
32b1e76
Update KahanSummation.jl
JeffreySarnoff Dec 11, 2017
863d46c
test sum_kbn(f, A)
JeffreySarnoff Dec 11, 2017
d345164
respond to comment
JeffreySarnoff Dec 11, 2017
ee5abb3
fixup
JeffreySarnoff Dec 11, 2017
423929c
correct for Method definition sum_kbn(Any, Any) overwrite
JeffreySarnoff Dec 11, 2017
3d93013
change name of testset
JeffreySarnoff Dec 11, 2017
733f0ea
combine test sets
JeffreySarnoff Dec 11, 2017
f8915eb
test identity
JeffreySarnoff Dec 11, 2017
39bc274
try to make coveralls happy
JeffreySarnoff Dec 11, 2017
ad48228
fold special case for identity into sum_kbn
JeffreySarnoff Dec 11, 2017
03c5bc2
Update KahanSummation.jl
JeffreySarnoff Dec 12, 2017
cbbcbcc
maybe ok
JeffreySarnoff Dec 12, 2017
5cbd33a
add more cumsum_kbr(A, dim) tests
JeffreySarnoff Dec 12, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/KahanSummation.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ function sum_kbn(A)
end

function sum_kbn(f::Function, A)
Copy link
Member

Choose a reason for hiding this comment

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

This needn't be a function; for example, one could also pass a type.

Copy link
Member Author

Choose a reason for hiding this comment

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

I know -- I do not know how to annotate Function or TypeConstructor. When I had it as sum_kbn(f, A) things got murky with sum_kbn(identity, A) and there was no clean dispatch resolution. What is the correct way to approach this while following the guide that sum_kbn(A) should be defined in terms of sum_kbn(identity, A)?

Copy link
Member

Choose a reason for hiding this comment

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

I think the issue there is that the definition with identity was backwards. It should be sum_kbn(A) = sub_kbn(identity, A), with sum_kbn(f, A) as the only long-form definition of the function. That should remove any ambiguities.

Copy link
Member Author

Choose a reason for hiding this comment

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

does the application of identity disappear always?

Copy link
Member Author

Choose a reason for hiding this comment

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

e.g. map(identity, [....])

Copy link
Member

Choose a reason for hiding this comment

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

Yes, the function call should get optimized out

f === identity && return sum_kbn(A)
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 backwards. sum_kbn(A) should be implemented in terms of sum_kbn(identity, A), not the other way around. This way there will be only one implementation of the sum.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mean that I should follow this pattern?

function sum_kbn(f, A)
     ...
end
sum_kbn(a) = sum_kbn(identity, A)

Copy link
Member Author

Choose a reason for hiding this comment

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

later comment says "yes" -- I will do that

T = @default_eltype(typeof(A))
c = promote_sys_size_add(zero(T)::T)
Copy link
Member

Choose a reason for hiding this comment

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

You'll need to apply the function here as well

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 the wrong type. The output of f might not match the eltype of A.

Copy link
Member Author

Choose a reason for hiding this comment

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

how does one obtain the f mapped eltype from eltype(A)?

i = start(A)
Expand All @@ -136,6 +137,4 @@ function sum_kbn(f::Function, A)
s - c
end

sum_kbn(identity, A) = sum_kbn(A)

end # module