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

Implemented segmented find and its variations for partitioned vector #2792

Merged
merged 55 commits into from Aug 14, 2017

Conversation

3 participants
@ajaivgeorge
Contributor

ajaivgeorge commented Aug 1, 2017

Tasks completed as part of GSoC project (See: #1338) -

  • Implemented segmented find.
  • Implemented segmented find_if.
  • Implemented segmented find_if_not.
  • The above use a common intermediate caller called segmented_find in segmented_algorithms/find.hpp
  • Passing unit tests for each of them have also been added.
  • Inspect shows 0 errors

ajaivgeorge added some commits Jun 22, 2017

Changed segmented dispatch algorithm result helper for 1 iterator to …
…return local_iterator instead of local_raw_iterator
@ajaivgeorge

This comment has been minimized.

Show comment
Hide comment
@ajaivgeorge

ajaivgeorge Aug 1, 2017

Contributor

@hkaiser and @mcopik, Please have a look.

Contributor

ajaivgeorge commented Aug 1, 2017

@hkaiser and @mcopik, Please have a look.

template <typename FwdIter>
struct find : public detail::algorithm<find<FwdIter>, FwdIter>
template <typename Iter>
struct find : public detail::algorithm<find<Iter>, Iter>

This comment has been minimized.

@mcopik

mcopik Aug 1, 2017

Contributor

Why change the iterator type name?

@mcopik

mcopik Aug 1, 2017

Contributor

Why change the iterator type name?

This comment has been minimized.

@ajaivgeorge

ajaivgeorge Aug 1, 2017

Contributor

I will change this back.

@ajaivgeorge

ajaivgeorge Aug 1, 2017

Contributor

I will change this back.

This comment has been minimized.

@ajaivgeorge

ajaivgeorge Aug 2, 2017

Contributor

I had changed the name because the template was clashing with the FwdIter declaration in parallel. I can keep this as is or change this to FwdIter and the one in parallel to FwdIter1.

@ajaivgeorge

ajaivgeorge Aug 2, 2017

Contributor

I had changed the name because the template was clashing with the FwdIter declaration in parallel. I can keep this as is or change this to FwdIter and the one in parallel to FwdIter1.

@@ -48,7 +48,7 @@ namespace hpx { namespace parallel { inline namespace v1
return std::find(first, last, val);
}
template <typename ExPolicy, typename T>
template <typename ExPolicy, typename FwdIter, typename T>

This comment has been minimized.

@mcopik

mcopik Aug 1, 2017

Contributor

The iterator type here is unnecessary, since it is determined by the template iterator type in the function object.

@mcopik

mcopik Aug 1, 2017

Contributor

The iterator type here is unnecessary, since it is determined by the template iterator type in the function object.

This comment has been minimized.

@ajaivgeorge

ajaivgeorge Aug 1, 2017

Contributor

Isn't the template of function object supposed to be local_vector_iterator and that of the input and output of sequential and parallel local_raw_vector_iterator?

@ajaivgeorge

ajaivgeorge Aug 1, 2017

Contributor

Isn't the template of function object supposed to be local_vector_iterator and that of the input and output of sequential and parallel local_raw_vector_iterator?

This comment has been minimized.

@mcopik

mcopik Aug 1, 2017

Contributor

You're right, my mistake - I forgot about that.

@mcopik

mcopik Aug 1, 2017

Contributor

You're right, my mistake - I forgot about that.

Show outdated Hide outdated hpx/parallel/algorithms/find.hpp
#endif
return detail::find<FwdIter>().call(
std::forward<ExPolicy>(policy), is_seq(),
first, last, std::move(val));

This comment has been minimized.

@mcopik

mcopik Aug 1, 2017

Contributor

Why is this value moved?

@mcopik

mcopik Aug 1, 2017

Contributor

Why is this value moved?

Show outdated Hide outdated hpx/parallel/segmented_algorithms/find.hpp
{
template <typename Algo, typename ExPolicy, typename InIter,
typename U>
inline typename std::enable_if<

This comment has been minimized.

@mcopik

mcopik Aug 1, 2017

Contributor

I'm not sure we need the enable_if here. This is an internal function in detail namespace and it is supposed to be called by an outer API function which performs the static verification already.

@mcopik

mcopik Aug 1, 2017

Contributor

I'm not sure we need the enable_if here. This is an internal function in detail namespace and it is supposed to be called by an outer API function which performs the static verification already.

Show outdated Hide outdated hpx/parallel/segmented_algorithms/find.hpp
template <typename Algo, typename ExPolicy, typename InIter,
typename U>
inline typename std::enable_if<

This comment has been minimized.

@mcopik

mcopik Aug 1, 2017

Contributor

Same as above.

@mcopik

mcopik Aug 1, 2017

Contributor

Same as above.

Show outdated Hide outdated hpx/parallel/segmented_algorithms/find.hpp
}
template <typename ExPolicy, typename InIter, typename T>
inline typename std::enable_if<

This comment has been minimized.

@mcopik

mcopik Aug 1, 2017

Contributor

In this function and all others - I think it's redundant to verify the execution policy everywhere. @hkaiser, what do you think?

@mcopik

mcopik Aug 1, 2017

Contributor

In this function and all others - I think it's redundant to verify the execution policy everywhere. @hkaiser, what do you think?

This comment has been minimized.

@hkaiser

hkaiser Aug 2, 2017

Member

@mcopik : I agree.

@hkaiser

hkaiser Aug 2, 2017

Member

@mcopik : I agree.

ajaivgeorge added some commits Aug 2, 2017

@ajaivgeorge

This comment has been minimized.

Show comment
Hide comment
@ajaivgeorge

ajaivgeorge Aug 2, 2017

Contributor

@hkaiser, @mcopik, I have removed the enable if from inner functions and have also changed the template name from InIter to FwdIter.

Contributor

ajaivgeorge commented Aug 2, 2017

@hkaiser, @mcopik, I have removed the enable if from inner functions and have also changed the template name from InIter to FwdIter.

@hkaiser hkaiser added this to Work in progress in Standard Algorithms Aug 8, 2017

@hkaiser

This comment has been minimized.

Show comment
Hide comment
@hkaiser

hkaiser Aug 12, 2017

Member

What's the status of this? Could we get an update, please?

Member

hkaiser commented Aug 12, 2017

What's the status of this? Could we get an update, please?

@ajaivgeorge

This comment has been minimized.

Show comment
Hide comment
@ajaivgeorge

ajaivgeorge Aug 12, 2017

Contributor

@hkaiser, I had fixed the raised review issues in the last commit. Please let me know if there are more issues.

Contributor

ajaivgeorge commented Aug 12, 2017

@hkaiser, I had fixed the raised review issues in the last commit. Please let me know if there are more issues.

@mcopik

This comment has been minimized.

Show comment
Hide comment
@mcopik

mcopik Aug 13, 2017

Contributor
Contributor

mcopik commented Aug 13, 2017

@hkaiser hkaiser merged commit 188c202 into STEllAR-GROUP:master Aug 14, 2017

2 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@hkaiser hkaiser moved this from Work in progress to Merged to master in Standard Algorithms Aug 14, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment