Skip to content

Commit a66084c

Browse files
authored
Merge pull request #7211 from aneek22112007-tech/fix/minmax-parallel-reduction-return-type
[parallel] Fix incorrect return types in max_element and minmax_element parallel reduction
2 parents 90bdaba + 48b26c6 commit a66084c

3 files changed

Lines changed: 265 additions & 26 deletions

File tree

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

Lines changed: 27 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) 2014-2025 Hartmut Kaiser
1+
// Copyright (c) 2014-2026 Hartmut Kaiser
22
//
33
// SPDX-License-Identifier: BSL-1.0
44
// Distributed under the Boost Software License, Version 1.0. (See accompanying
@@ -365,6 +365,7 @@ namespace hpx {
365365

366366
#include <hpx/config.hpp>
367367
#include <hpx/algorithms/traits/is_value_proxy.hpp>
368+
#include <hpx/algorithms/traits/projected.hpp>
368369
#include <hpx/assert.hpp>
369370
#include <hpx/modules/concepts.hpp>
370371
#include <hpx/modules/executors.hpp>
@@ -403,11 +404,11 @@ namespace hpx::parallel {
403404
if (count == 0 || count == 1)
404405
return it;
405406

406-
using element_type = hpx::traits::proxy_value_t<
407-
typename std::iterator_traits<FwdIter>::value_type>;
408-
409407
auto smallest = it;
410408

409+
using element_type = hpx::traits::proxy_value_t<
410+
std::decay_t<decltype(HPX_INVOKE(proj, *smallest))>>;
411+
411412
element_type value = HPX_INVOKE(proj, *smallest);
412413
util::loop_n<std::decay_t<ExPolicy>>(
413414
++it, count - 1, [&](FwdIter const& curr) -> void {
@@ -442,9 +443,8 @@ namespace hpx::parallel {
442443

443444
auto smallest = *it;
444445

445-
using element_type =
446-
hpx::traits::proxy_value_t<typename std::iterator_traits<
447-
decltype(smallest)>::value_type>;
446+
using element_type = hpx::traits::proxy_value_t<
447+
std::decay_t<decltype(HPX_INVOKE(proj, *smallest))>>;
448448

449449
element_type value = HPX_INVOKE(proj, *smallest);
450450
util::loop_n<std::decay_t<ExPolicy>>(
@@ -473,11 +473,11 @@ namespace hpx::parallel {
473473
if (first == last)
474474
return first;
475475

476-
using element_type = hpx::traits::proxy_value_t<
477-
typename std::iterator_traits<FwdIter>::value_type>;
478-
479476
auto smallest = first;
480477

478+
using element_type = hpx::traits::proxy_value_t<
479+
std::decay_t<decltype(HPX_INVOKE(proj, *smallest))>>;
480+
481481
element_type value = HPX_INVOKE(proj, *smallest);
482482
util::loop(HPX_FORWARD(ExPolicy, policy), ++first, last,
483483
[&](FwdIter const& curr) -> void {
@@ -556,11 +556,11 @@ namespace hpx::parallel {
556556
if (count == 0 || count == 1)
557557
return it;
558558

559-
using element_type = hpx::traits::proxy_value_t<
560-
typename std::iterator_traits<FwdIter>::value_type>;
561-
562559
auto largest = it;
563560

561+
using element_type = hpx::traits::proxy_value_t<
562+
std::decay_t<decltype(HPX_INVOKE(proj, *largest))>>;
563+
564564
element_type value = HPX_INVOKE(proj, *largest);
565565
util::loop_n<std::decay_t<ExPolicy>>(
566566
++it, count - 1, [&](FwdIter const& curr) -> void {
@@ -583,7 +583,8 @@ namespace hpx::parallel {
583583
// generically from the segmented algorithms
584584
template <typename ExPolicy, typename FwdIter, typename F,
585585
typename Proj>
586-
static constexpr typename std::iterator_traits<FwdIter>::value_type
586+
static constexpr hpx::traits::proxy_value_t<
587+
typename std::iterator_traits<FwdIter>::value_type>
587588
sequential_minmax_element_ind(ExPolicy&&, FwdIter it,
588589
std::size_t count, F const& f, Proj const& proj)
589590
{
@@ -594,9 +595,8 @@ namespace hpx::parallel {
594595

595596
auto largest = *it;
596597

597-
using element_type =
598-
hpx::traits::proxy_value_t<typename std::iterator_traits<
599-
decltype(largest)>::value_type>;
598+
using element_type = hpx::traits::proxy_value_t<
599+
std::decay_t<decltype(HPX_INVOKE(proj, *largest))>>;
600600

601601
element_type value = HPX_INVOKE(proj, *largest);
602602
util::loop_n<std::decay_t<ExPolicy>>(
@@ -625,11 +625,11 @@ namespace hpx::parallel {
625625
if (first == last)
626626
return first;
627627

628-
using element_type = hpx::traits::proxy_value_t<
629-
typename std::iterator_traits<FwdIter>::value_type>;
630-
631628
auto largest = first;
632629

630+
using element_type = hpx::traits::proxy_value_t<
631+
std::decay_t<decltype(HPX_INVOKE(proj, *largest))>>;
632+
633633
element_type value = HPX_INVOKE(proj, *largest);
634634
util::loop(HPX_FORWARD(ExPolicy, policy), ++first, last,
635635
[&](FwdIter const& curr) -> void {
@@ -711,7 +711,7 @@ namespace hpx::parallel {
711711
return result;
712712

713713
using element_type = hpx::traits::proxy_value_t<
714-
typename std::iterator_traits<FwdIter>::value_type>;
714+
std::decay_t<decltype(HPX_INVOKE(proj, *it))>>;
715715

716716
element_type min_value = HPX_INVOKE(proj, *it);
717717
element_type max_value = min_value;
@@ -742,7 +742,8 @@ namespace hpx::parallel {
742742
// generically from the segmented algorithms
743743
template <typename ExPolicy, typename PairIter, typename F,
744744
typename Proj>
745-
static typename std::iterator_traits<PairIter>::value_type
745+
static constexpr hpx::traits::proxy_value_t<
746+
typename std::iterator_traits<PairIter>::value_type>
746747
sequential_minmax_element_ind(ExPolicy&&, PairIter it,
747748
std::size_t count, F const& f, Proj const& proj)
748749
{
@@ -751,11 +752,11 @@ namespace hpx::parallel {
751752
if (count == 1)
752753
return *it;
753754

754-
using element_type = hpx::traits::proxy_value_t<
755-
typename std::iterator_traits<Iter>::value_type>;
756-
757755
auto result = *it;
758756

757+
using element_type = hpx::traits::proxy_value_t<
758+
std::decay_t<decltype(HPX_INVOKE(proj, *result.min))>>;
759+
759760
element_type min_value = HPX_INVOKE(proj, *result.min);
760761
element_type max_value = HPX_INVOKE(proj, *result.max);
761762
util::loop_n<std::decay_t<ExPolicy>>(
@@ -800,7 +801,7 @@ namespace hpx::parallel {
800801
}
801802

802803
using element_type = hpx::traits::proxy_value_t<
803-
typename std::iterator_traits<FwdIter>::value_type>;
804+
std::decay_t<decltype(HPX_INVOKE(proj, *min))>>;
804805

805806
element_type min_value = HPX_INVOKE(proj, *min);
806807
element_type max_value = HPX_INVOKE(proj, *max);

libs/core/algorithms/tests/unit/algorithms/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ set(tests
8787
merge
8888
min_element
8989
minmax_element
90+
minmax_element_parallel_projection
9091
mismatch
9192
mismatch_binary
9293
move
Lines changed: 237 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,237 @@
1+
// Copyright (c) 2024-2026 STE||AR-Group
2+
//
3+
// SPDX-License-Identifier: BSL-1.0
4+
// Distributed under the Boost Software License, Version 1.0. (See accompanying
5+
// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)
6+
7+
// Regression test for: incorrect return types in
8+
// max_element::sequential_minmax_element_ind and
9+
// minmax_element::sequential_minmax_element_ind, which are called exclusively
10+
// in the parallel reduction phase. This file exercises the projection + par
11+
// path to ensure the parallel aggregation returns an iterator (not a copy).
12+
13+
#include <hpx/algorithm.hpp>
14+
#include <hpx/execution.hpp>
15+
#include <hpx/init.hpp>
16+
#include <hpx/modules/testing.hpp>
17+
18+
#include <algorithm>
19+
#include <cstddef>
20+
#include <ctime>
21+
#include <functional>
22+
#include <iostream>
23+
#include <string>
24+
#include <utility>
25+
#include <vector>
26+
27+
// ---------------------------------------------------------------------------
28+
// test_min_element_parallel_projection
29+
// Verifies that hpx::ranges::min_element with par and a projection correctly
30+
// identifies the minimum by projected value, exercising the parallel
31+
// reduction path (sequential_minmax_element_ind returns Iter).
32+
// ---------------------------------------------------------------------------
33+
template <typename ExPolicy>
34+
void test_min_element_parallel_projection(ExPolicy policy)
35+
{
36+
// Pairs: compare by .second (the projected key)
37+
std::vector<std::pair<int, int>> c = {
38+
{10, 5}, {20, 1}, {30, 8}, {40, 3}, {50, 7}};
39+
40+
auto proj = [](std::pair<int, int> const& p) { return p.second; };
41+
42+
auto it = hpx::ranges::min_element(
43+
policy, c.begin(), c.end(), std::less<int>{}, proj);
44+
45+
HPX_TEST(it != c.end());
46+
// Minimum second value is 1, belonging to {20, 1}
47+
HPX_TEST_EQ(it->second, 1);
48+
HPX_TEST_EQ(it->first, 20);
49+
}
50+
51+
// ---------------------------------------------------------------------------
52+
// test_max_element_parallel_projection
53+
// Verifies that hpx::ranges::max_element with par and a projection correctly
54+
// identifies the maximum by projected value, exercising the parallel
55+
// reduction path (sequential_minmax_element_ind returns Iter, not value_type).
56+
// ---------------------------------------------------------------------------
57+
template <typename ExPolicy>
58+
void test_max_element_parallel_projection(ExPolicy policy)
59+
{
60+
std::vector<std::pair<int, int>> c = {
61+
{10, 5}, {20, 1}, {30, 8}, {40, 3}, {50, 7}};
62+
63+
auto proj = [](std::pair<int, int> const& p) { return p.second; };
64+
65+
auto it = hpx::ranges::max_element(
66+
policy, c.begin(), c.end(), std::less<int>{}, proj);
67+
68+
HPX_TEST(it != c.end());
69+
// Maximum second value is 8, belonging to {30, 8}
70+
HPX_TEST_EQ(it->second, 8);
71+
HPX_TEST_EQ(it->first, 30);
72+
}
73+
74+
// ---------------------------------------------------------------------------
75+
// test_minmax_element_parallel_projection
76+
// Verifies that hpx::ranges::minmax_element with par and a projection correctly
77+
// identifies both min and max by projected value, exercising the parallel
78+
// reduction path (sequential_minmax_element_ind returns minmax_element_result<Iter>).
79+
// ---------------------------------------------------------------------------
80+
template <typename ExPolicy>
81+
void test_minmax_element_parallel_projection(ExPolicy policy)
82+
{
83+
std::vector<std::pair<int, int>> c = {
84+
{10, 5}, {20, 1}, {30, 8}, {40, 3}, {50, 7}};
85+
86+
auto proj = [](std::pair<int, int> const& p) { return p.second; };
87+
88+
auto result = hpx::ranges::minmax_element(
89+
policy, c.begin(), c.end(), std::less<int>{}, proj);
90+
91+
HPX_TEST(result.min != c.end());
92+
HPX_TEST(result.max != c.end());
93+
94+
// Min by second value: 1 => {20, 1}
95+
HPX_TEST_EQ(result.min->second, 1);
96+
HPX_TEST_EQ(result.min->first, 20);
97+
98+
// Max by second value: 8 => {30, 8}
99+
HPX_TEST_EQ(result.max->second, 8);
100+
HPX_TEST_EQ(result.max->first, 30);
101+
}
102+
103+
// ---------------------------------------------------------------------------
104+
// Larger dataset tests: forces actual parallel partitioning to occur,
105+
// directly triggering the sequential_minmax_element_ind reduction path.
106+
// ---------------------------------------------------------------------------
107+
template <typename ExPolicy>
108+
void test_min_element_parallel_projection_large(ExPolicy policy)
109+
{
110+
// 10000 pairs: projected key is index % 97, minimum is at index 0 (key 0)
111+
std::size_t const N = 10000;
112+
std::vector<std::pair<std::size_t, int>> c;
113+
c.reserve(N);
114+
for (std::size_t i = 0; i < N; ++i)
115+
c.push_back({i, static_cast<int>(i % 97)});
116+
117+
// Reference: minimum projected value is 0
118+
auto proj = [](std::pair<std::size_t, int> const& p) { return p.second; };
119+
auto ref_it = std::min_element(c.begin(), c.end(),
120+
[&proj](auto const& a, auto const& b) { return proj(a) < proj(b); });
121+
122+
auto it = hpx::ranges::min_element(
123+
policy, c.begin(), c.end(), std::less<int>{}, proj);
124+
125+
HPX_TEST(it != c.end());
126+
HPX_TEST_EQ(it->second, ref_it->second);
127+
}
128+
129+
template <typename ExPolicy>
130+
void test_max_element_parallel_projection_large(ExPolicy policy)
131+
{
132+
std::size_t const N = 10000;
133+
std::vector<std::pair<std::size_t, int>> c;
134+
c.reserve(N);
135+
for (std::size_t i = 0; i < N; ++i)
136+
c.push_back({i, static_cast<int>(i % 97)});
137+
138+
auto proj = [](std::pair<std::size_t, int> const& p) { return p.second; };
139+
auto ref_it = std::max_element(c.begin(), c.end(),
140+
[&proj](auto const& a, auto const& b) { return proj(a) < proj(b); });
141+
142+
auto it = hpx::ranges::max_element(
143+
policy, c.begin(), c.end(), std::less<int>{}, proj);
144+
145+
HPX_TEST(it != c.end());
146+
HPX_TEST_EQ(it->second, ref_it->second);
147+
}
148+
149+
template <typename ExPolicy>
150+
void test_minmax_element_parallel_projection_large(ExPolicy policy)
151+
{
152+
std::size_t const N = 10000;
153+
std::vector<std::pair<std::size_t, int>> c;
154+
c.reserve(N);
155+
for (std::size_t i = 0; i < N; ++i)
156+
c.push_back({i, static_cast<int>(i % 97)});
157+
158+
auto proj = [](std::pair<std::size_t, int> const& p) { return p.second; };
159+
160+
auto ref_min = std::min_element(c.begin(), c.end(),
161+
[&proj](auto const& a, auto const& b) { return proj(a) < proj(b); });
162+
auto ref_max = std::max_element(c.begin(), c.end(),
163+
[&proj](auto const& a, auto const& b) { return proj(a) < proj(b); });
164+
165+
auto result = hpx::ranges::minmax_element(
166+
policy, c.begin(), c.end(), std::less<int>{}, proj);
167+
168+
HPX_TEST(result.min != c.end());
169+
HPX_TEST(result.max != c.end());
170+
HPX_TEST_EQ(result.min->second, ref_min->second);
171+
HPX_TEST_EQ(result.max->second, ref_max->second);
172+
}
173+
174+
///////////////////////////////////////////////////////////////////////////////
175+
void minmax_parallel_projection_test()
176+
{
177+
using namespace hpx::execution;
178+
179+
// Small dataset: tests correct iterator return value
180+
test_min_element_parallel_projection(seq);
181+
test_min_element_parallel_projection(par);
182+
test_min_element_parallel_projection(par_unseq);
183+
184+
test_max_element_parallel_projection(seq);
185+
test_max_element_parallel_projection(par);
186+
test_max_element_parallel_projection(par_unseq);
187+
188+
test_minmax_element_parallel_projection(seq);
189+
test_minmax_element_parallel_projection(par);
190+
test_minmax_element_parallel_projection(par_unseq);
191+
192+
// Large dataset: triggers actual parallel partitioning and reduction
193+
test_min_element_parallel_projection_large(seq);
194+
test_min_element_parallel_projection_large(par);
195+
196+
test_max_element_parallel_projection_large(seq);
197+
test_max_element_parallel_projection_large(par);
198+
199+
test_minmax_element_parallel_projection_large(seq);
200+
test_minmax_element_parallel_projection_large(par);
201+
}
202+
203+
///////////////////////////////////////////////////////////////////////////////
204+
int hpx_main(hpx::program_options::variables_map& vm)
205+
{
206+
unsigned int seed = static_cast<unsigned int>(std::time(nullptr));
207+
if (vm.count("seed"))
208+
seed = vm["seed"].as<unsigned int>();
209+
210+
std::cout << "using seed: " << seed << std::endl;
211+
std::srand(seed);
212+
213+
minmax_parallel_projection_test();
214+
215+
return hpx::local::finalize();
216+
}
217+
218+
int main(int argc, char* argv[])
219+
{
220+
using namespace hpx::program_options;
221+
options_description desc_commandline(
222+
"Usage: " HPX_APPLICATION_STRING " [options]");
223+
224+
desc_commandline.add_options()("seed,s", value<unsigned int>(),
225+
"the random number generator seed to use for this run");
226+
227+
std::vector<std::string> const cfg = {"hpx.os_threads=all"};
228+
229+
hpx::local::init_params init_args;
230+
init_args.desc_cmdline = desc_commandline;
231+
init_args.cfg = cfg;
232+
233+
HPX_TEST_EQ_MSG(hpx::local::init(hpx_main, argc, argv, init_args), 0,
234+
"HPX main exited with non-zero status");
235+
236+
return hpx::util::report_errors();
237+
}

0 commit comments

Comments
 (0)