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

Reductions with defaulted identity value #343

Closed
jdgarciauc3m opened this issue Mar 2, 2018 · 5 comments
Closed

Reductions with defaulted identity value #343

jdgarciauc3m opened this issue Mar 2, 2018 · 5 comments
Assignees
Milestone

Comments

@jdgarciauc3m
Copy link
Contributor

Add an additional family of reductions without identity value only for those types that are default constructible. That version assumes that the default value for the type is the identity value. Not that this is consistent with addition in numeric types.

This issue partiallly solves #290

@jdgarciauc3m
Copy link
Contributor Author

@droccom has raised in comments at issue #290 if we should provide reduce either with an identity value or an initial value. Rationale is being aligned with std::reduce

We need to discuss this in detail to make a decision.

@mdrocco
Copy link
Contributor

mdrocco commented Mar 4, 2018

If the target is strict consistency with std::reduce, there should be no way to both avoiding the initial value and using a custom binary operator (only std::plus<> is supported).

I think this case is very specific, so I suggest to either:
a. avoid adding it to the GrPPI API;
b. extend it to support generic binary operators (relaxing consistency with std::reduce)

@jdgarciauc3m
Copy link
Contributor Author

New proposed resolution:

Do not take initial value or identity for reduction operations and apply the following semantics:

  • reduce(ex, b, e, binop) -> if (distance(b,e) = 1) return *b
  • reduce(ex, b, e, binop) -> Apply reduction over range [b,e)
  • reduce(ex, b, e, binop) -> if (distance(b,e) <1) return default constructed value

Requirements:

  • value type must be default constructible
  • The result is indeterminate if either binop is not associative or not commutative.

@mdrocco
Copy link
Contributor

mdrocco commented Mar 22, 2018

This solution makes great sense to me.

@jdgarciauc3m jdgarciauc3m modified the milestones: v0.4.1, v0.4.2 May 15, 2018
@jdgarciauc3m
Copy link
Contributor Author

We keep the requirement of having to provide identity_value.

Rationale: This is in-line with new reduction objects of parallelism TS in C++. Fastflow also requires an identity value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Interface evolution
Awaiting triage
Development

No branches or pull requests

3 participants