Contains and contains_subrange parallel algorithm implementation GSOC 2024 #6497

Zak-K-Abdi · 2024-05-29T17:56:53Z

Implementing parallel algorithms for contains and contains_subrange. GSOC 2024 proposal.
Will attempt to fix hpx::search so hpx::contains_subrange can make a call to hpx::search.

jenkins-cscs · 2024-05-29T17:58:27Z

Can one of the admins verify this patch?

Pansysk75 · 2024-05-29T18:31:16Z

libs/core/algorithms/include/hpx/parallel/algorithms/contains.hpp

+            template <typename ExPolicy, typename Iterator, typename Sentinel,
+                typename T, typename Proj>
+            static util::detail::algorithm_result_t<ExPolicy, bool> parallel(
+                ExPolicy&& policy, Iterator first, Sentinel last, const T& val,
+                Proj&& proj)
+            {
+                const auto distance = detail::distance(first, last);
+                if (distance <= 0)
+                {
+                    return util::detail::algorithm_result<ExPolicy, bool>::get(
+                        false);
+                }
+
+                const auto itr = util::loop_pred<
+                    std::decay_t<ExPolicy>>(first, last, [&val, &proj]
+                    (const auto& cur) { return HPX_INVOKE(proj, *cur) == val; });
+
+                if (itr == last)
+                {
+                    return util::detail::algorithm_result<ExPolicy, bool>::get(
+                        false);
+                }
+
+                return util::detail::algorithm_result<ExPolicy, bool>::get(true);
+            }


This is probably work-in-progress, but I just want to mention that util::loop_pred doesn't actually use the execution policy to run in parallel, so this will not run in multiple threads. You'll have to use a partitioner, which splits the work and launches it on multiple (the equal algorithm is a a nice example).

Ah I was unaware at the time I wrote that, I should have been more mindful of what I was using. I checked out equal algorithm and I made some changes that now use the partitioner in the parallel overload. Also, I was hesitant to do this but I created a file in the parallel/algorithms/detail called contains.hpp, holding the implementation details for the algorithm.

codacy-production · 2024-05-29T22:08:10Z

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
✅ -0.10%	✅ 90.96%

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (`f4ff6e8`)	232431	198777	85.52%
Head commit (`1b22fae`)	205360 (-27071)	175429 (-23348)	85.43% (-0.10%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#6497)	509	463	90.96%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings Change summary preferences

_{Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more}

hkaiser · 2024-06-03T12:13:24Z

@Zak-K-Abdi: Please take a look at the clang-format problems (https://gist.github.com/hkaiser/26bced03497017f481dbb9ee7d0df67c) and the cmake issues:

Missing hpx/parallel/algorithms/detail/contains.hpp in libs/core/algorithms/CMakeLists.txt

Also here is the list of inspect issues reported:

/libs/core/algorithms/include/hpx/parallel/algorithms/contains.hpp: END file doesn't end with a newline, Endline Whitespace: 74, C, Lic, SPDX-Lic
/libs/core/algorithms/include/hpx/parallel/algorithms/detail/contains.hpp: END file doesn't end with a newline, C, Lic, SPDX-Lic

and tests for both algorithms.

rather than using .get()

for cleaner code

Zak-K-Abdi · 2024-09-04T19:55:12Z

libs/core/algorithms/include/hpx/parallel/container_algorithms/contains.hpp

+        static hpx::future<bool> get(hpx::future<T>& itr, T& last)
+        {
+            return itr.then(
+                [&last](hpx::future<T> it) { return it.get() != last; });


@Pansysk75 Hi Panos, I'm not sure if this is correct, could you check this line of code I changed? I changed the helper function to return hpx::future, a continuation to the previous future, by using .then().

That looks right, i think you might want to capture last by value and not by reference, as it is a local variable of the parallel function. Otherwise looks good!

@Pansysk75 Just to make sure, should I also pass last by value for the functions in my contains_subrange_helper_class or only for the lambda?

Pansysk75 · 2024-09-04T22:53:49Z

libs/core/algorithms/include/hpx/parallel/container_algorithms/contains.hpp

@@ -148,8 +137,7 @@ namespace hpx::parallel { namespace detail {
                HPX_FORWARD(Proj2, proj2));

            return util::detail::algorithm_result<ExPolicy, bool>::get(
-                contains_subrange_helper<ExPolicy, FwdIter1>().get(itr) !=
-                last1);
+                contains_subrange_helper<ExPolicy, FwdIter1>().get(itr, last1));


I think returning contains_subrange_helper<ExPolicy, FwdIter1>().get(itr, last1) would suffice here.
The return types of your helper function already match with algorithm_result_t. Calling that additional ::get will call .get() on your future before returning it, which undermines your latest change on your helper.

Pansysk75 · 2024-09-25T19:28:51Z

@Zak-K-Abdi Could you please address this: #6497 (comment)
Otherwise this looks ready to merge, thanks for your efforts!

Zak-K-Abdi · 2024-09-26T09:26:54Z

@Zak-K-Abdi Could you please address this: #6497 (comment) Otherwise this looks ready to merge, thanks for your efforts!

Apologies, I completely forgot about that comment. I will make that change soon. Thanks.

hkaiser

LGTM, thanks!

Zak-K-Abdi requested a review from hkaiser as a code owner May 29, 2024 17:56

Pansysk75 added type: enhancement category: algorithms labels May 29, 2024

hkaiser added this to the 1.11.0 milestone May 29, 2024

Pansysk75 reviewed May 29, 2024

View reviewed changes

Pansysk75 changed the title ~~Contains and contains_subrange parallel algorithm implementaion GSOC 2024~~ Contains and contains_subrange parallel algorithm implementation GSOC 2024 May 29, 2024

Zak-K-Abdi force-pushed the contains/contains_subrange branch from 1f8897c to f2496c9 Compare June 2, 2024 17:32

Zak-K-Abdi force-pushed the contains/contains_subrange branch 5 times, most recently from 7b69479 to 711ca44 Compare June 11, 2024 17:21

Zak-K-Abdi force-pushed the contains/contains_subrange branch from 711ca44 to 38017ef Compare June 15, 2024 07:16

Contains and contains_subrange parallel algorithm implementaion

54b3d7a

Zak-K-Abdi force-pushed the contains/contains_subrange branch from 38017ef to 54b3d7a Compare June 15, 2024 07:27

Zak-K-Abdi added 7 commits August 6, 2024 17:26

contains_subrange implementation

6c9b892

hpx::ranges::contains and hpx::ranges::contains_subrange

3f05c8d

and tests for both algorithms.

clang format changes

adad59d

Attempting to fix broken tests

74b55e4

Implement a continuation to the future using .then()

6f7f303

rather than using .get()

turning clang format off for some function templates

2f8fe86

for cleaner code

Fix failing circleci test

f04a626

Zak-K-Abdi commented Sep 4, 2024

View reviewed changes

Pansysk75 reviewed Sep 4, 2024

View reviewed changes

Use helper function rather than algorithm_result::get

bdbd643

Zak-K-Abdi force-pushed the contains/contains_subrange branch from 125867c to bdbd643 Compare September 5, 2024 18:36

Merge branch 'master' into contains/contains_subrange

1b22fae

Capture last by value instead of by reference

87f72bd

hkaiser approved these changes Oct 8, 2024

View reviewed changes

hkaiser merged commit 6e59829 into STEllAR-GROUP:master Oct 8, 2024
71 of 80 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Contains and contains_subrange parallel algorithm implementation GSOC 2024 #6497

Contains and contains_subrange parallel algorithm implementation GSOC 2024 #6497

Zak-K-Abdi commented May 29, 2024 •

edited by hkaiser

Loading

jenkins-cscs commented May 29, 2024

Pansysk75 May 29, 2024

Zak-K-Abdi Jun 2, 2024 •

edited

Loading

codacy-production bot commented May 29, 2024 •

edited

Loading

hkaiser commented Jun 3, 2024

Zak-K-Abdi Sep 4, 2024

Pansysk75 Sep 4, 2024

Zak-K-Abdi Sep 26, 2024

Pansysk75 Sep 4, 2024

Pansysk75 commented Sep 25, 2024

Zak-K-Abdi commented Sep 26, 2024

hkaiser left a comment

Contains and contains_subrange parallel algorithm implementation GSOC 2024 #6497

Contains and contains_subrange parallel algorithm implementation GSOC 2024 #6497

Conversation

Zak-K-Abdi commented May 29, 2024 • edited by hkaiser Loading

jenkins-cscs commented May 29, 2024

Pansysk75 May 29, 2024

Choose a reason for hiding this comment

Zak-K-Abdi Jun 2, 2024 • edited Loading

Choose a reason for hiding this comment

codacy-production bot commented May 29, 2024 • edited Loading

Coverage summary from Codacy

See diff coverage on Codacy

See your quality gate settings Change summary preferences

hkaiser commented Jun 3, 2024

Zak-K-Abdi Sep 4, 2024

Choose a reason for hiding this comment

Pansysk75 Sep 4, 2024

Choose a reason for hiding this comment

Zak-K-Abdi Sep 26, 2024

Choose a reason for hiding this comment

Pansysk75 Sep 4, 2024

Choose a reason for hiding this comment

Pansysk75 commented Sep 25, 2024

Zak-K-Abdi commented Sep 26, 2024

hkaiser left a comment

Choose a reason for hiding this comment

Zak-K-Abdi commented May 29, 2024 •

edited by hkaiser

Loading

Zak-K-Abdi Jun 2, 2024 •

edited

Loading

codacy-production bot commented May 29, 2024 •

edited

Loading