Skip to content

Conversation

vug
Copy link

@vug vug commented Mar 12, 2023

Hi! while watching your talk on YouTube ran this logic on a small example and it didn't give me the correct result.

https://en.cppreference.com/w/cpp/algorithm/reduce says the binary operation has to be associative and commutative. I think a + (b == val) is not. Because the way reduce goes over a vector {a, b, c, d} with init is: R(R(R(a, b), R(c, d)), init), NOT R(R(R(R(init, a), b), c), d). :-O I realized this when I saw the diagram for reduce in https://blog.tartanllama.xyz/accumulate-vs-reduce/.

I watched further into your talk and learned about transform_reduce. ^_^ Using that we can make the binary op of reduce part of transform_reduce commutative and associative.

Also I'm not an export. The way reduce goes over a vector could be compiler, platform dependent. Here is a Compiler Explorer comparison of reduce and transform_reduce versions: https://godbolt.org/z/r66M4cYsb

Have a nice day!

Hi! while watching your talk on YouTube ran this logic on a small example and it didn't give me the correct result.

https://en.cppreference.com/w/cpp/algorithm/reduce says the binary operation has to be associative and commutative. I think `a + (b == val)` is not. Because the way reduce goes over a vector `{a, b, c, d}` with `init` is: `R(R(R(a, b), R(c, d)), init)`, NOT `R(R(R(R(init, a), b), c), d)`.  :-O I realized this when I saw the diagram for reduce in https://blog.tartanllama.xyz/accumulate-vs-reduce/.

I watched further into your talk and learned about `transform_reduce`. ^_^ Using that we can make the binary op of `reduce` part of `transform_reduce` commutative and associative.

Also I'm not an export. The way reduce goes over a vector could be compiler, platform dependent. Here is a Compiler Explorer comparison of `reduce` and `transform_reduce` versions: https://godbolt.org/z/r66M4cYsb

Have a nice day!
@vug
Copy link
Author

vug commented Mar 12, 2023

This might break the flow of your presentation though. :-( Your favorite algorithm transform_reduce should come later in the presentation. :-)
Also, I created this pull request using the editor in GitHub's web interface. Might have written something wrong. Please check twice. ^_^

@vug
Copy link
Author

vug commented Mar 12, 2023

By the way, one other thing I've tried before switching to transform_reduce was to force reduce to operate like R(R(R(R(init, a), b), c), d), and used std::reduce(std::execution::seq, ... explicitly, but that didn't help either. (You can see that in the compiler explorer link above).

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.

1 participant