Implement minmax_element to use less comparisons

[Reboot-Complete]

Proposed Changes

• Improved hpx::minmax_element by now using floor(3/2*(N-1)) comparisons instead of 2*(N-1) comparisons.
• hpx::minmax_element no longer has contradictory information in HPX documentation versus its actual implementation.

Any background context you want to provide?

The algorithm std::minmax_element returns the first reference to the smallest element and the last reference to the largest element of a container using floor(3/2*(N-1)) comparisons. HPX documentation states that hpx::minmax_element has the same complexity; however, the current implementation actually uses 2*(N-1) comparisons by explicitly comparing the known minimum and maximum values against each individual element in the container.

This pull request reimplements hpx::minmax_element so that all execution policies now use floor(3/2*(N-1)) comparisons. By now iterating over two elements at once, comparing those elements to find which is smaller and which is larger, and then comparing those respective elements to the known minimum and known maximum, there are now three comparisons for every two elements rather than four comparisons for every two elements.

Checklist

Not all points below apply to all pull requests.

• I have added a new feature and have added tests to go along with it.
• I have fixed a bug and have added a regression test.
• I have added a test using random numbers; I have made sure it uses a seed, and that random numbers generated are valid inputs for the tests.

[StellarBot]

Can one of the admins verify this patch?

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• Performance

  • Improved sequential min/max element searches by processing elements in pairs, reducing comparisons while preserving results.
  • Added efficient handling for empty, single-element, and odd-length ranges.

• Bug Fixes

  • Expanded coverage for custom element types and value-based comparators across iterator and execution-policy overloads.

Walkthrough

The sequential minmax_element implementations now process elements pairwise across standalone and algorithm-object overloads, with explicit handling for empty, single-element, and odd-sized ranges. Tests add custom-type comparator coverage for iterator and execution-policy calls.

Changes

Pairwise minmax_element refactor

Layer / File(s)	Summary
Sequential minmax_element pairwise rewrite libs/core/algorithms/include/hpx/parallel/algorithms/minmax.hpp	Projected and identity sequential_minmax_element overloads replace element-by-element scans with pairwise initialization and updates, including trailing-single-element handling.
detail::minmax_element sequential pairwise rewrite libs/core/algorithms/include/hpx/parallel/algorithms/minmax.hpp	Projected and identity algorithm-object overloads use pairwise traversal after empty/single-element checks and handle an unpaired final element explicitly.
Custom comparator coverage libs/core/algorithms/tests/unit/algorithms/minmax_element.cpp	Iterator-only and execution-policy tests compare custom set results with std::minmax_element, then repeat after changing input values.

Estimated code review effort: 3 (Moderate) | ~25 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: reducing comparisons in minmax_element.
Description check	✅ Passed	The description matches the patch by describing the comparison reduction, documentation alignment, and regression test.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[coderabbitai]

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@libs/core/algorithms/include/hpx/parallel/algorithms/minmax.hpp`:
- Around line 1237-1300: The `detail::minmax_element::sequential` identity
overload has the same tie-breaking regression in its pair comparison logic,
where the strict `HPX_INVOKE(f, curr_value, next_value)` path can mishandle
equal elements and violate the first-smallest/last-largest guarantee. Update the
pair-processing branch in `minmax.hpp` so equal pairs are treated consistently
with the intended tie-breaking behavior, and make sure the `min`/`max` updates
in the `sequential` implementation preserve the correct earliest minimum and
latest maximum selection.
- Around line 1153-1216: The projected comparator path in
detail::minmax_element::sequential still has the same tie-breaking regression
when comparing curr_value and next_value. Update the pair-processing logic in
minmax_element::sequential so equal elements are handled consistently with the
non-regressed behavior, ensuring min/max updates do not depend on the strict
curr-vs-next ordering alone. Use the existing min, max, curr_value, next_value,
and HPX_INVOKE(f, ...) flow in this function to apply the same fix as the other
branch.
- Around line 976-1040: The identity overload in the minmax routine has the same
tie-breaking bug as the projected version: the paired comparison in the minmax
loop currently uses a strict order check in the pair test, which makes equal
elements choose the wrong min/max candidate. Update the logic in the minmax
implementation around the `HPX_INVOKE(f, curr_value, next_value)` branch so
equal-valued pairs preserve first-smallest for `result.min` and last-largest for
`result.max`, matching the contract used by the surrounding
`minmax`/`result.min`/`result.max` handling.
- Around line 892-956: The pairwise comparison logic in minmax_element is using
the wrong tie-break when values are equal, which can swap the earlier/later
iterator selection. Update the comparisons in the minmax implementation in
minmax.hpp so that pair ordering is decided with the reverse-argument check,
using the existing helpers around HPX_INVOKE(f, ...) in the minmax_element path.
Apply the same correction to the other three pairwise comparison sites in this
file so the first-smallest / last-largest behavior is preserved consistently.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3e9f7b57-aa87-488f-b9e1-0c171f8cecd7

📥 Commits

Reviewing files that changed from the base of the PR and between be9d087 and 4c69a24.

📒 Files selected for processing (1)

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

[hkaiser]

Could you please add the tests you created to verify correctness of strict-weak orderings?

[coderabbitai]

🧹 Nitpick comments (1)

libs/core/algorithms/tests/unit/algorithms/minmax_element.cpp (1)

67-94: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Assert the optimized comparison count.

These tests validate results but still pass with the old 2*(N-1) implementation. Add a counting comparator for sequential odd and even ranges and assert at most 3 * (N - 1) / 2 calls, directly protecting this PR’s primary objective.

Also applies to: 147-174

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@libs/core/algorithms/tests/unit/algorithms/minmax_element.cpp` around lines
67 - 94, Add a counting comparator to the minmax_element tests covering both
sequential odd- and even-sized ranges, and assert the comparison count is at
most 3 * (N - 1) / 2. Apply the same assertions to the additional test block
around the second referenced range while preserving the existing result checks
and comparator behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@libs/core/algorithms/tests/unit/algorithms/minmax_element.cpp`:
- Around line 67-94: Add a counting comparator to the minmax_element tests
covering both sequential odd- and even-sized ranges, and assert the comparison
count is at most 3 * (N - 1) / 2. Apply the same assertions to the additional
test block around the second referenced range while preserving the existing
result checks and comparator behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: efe0cd63-24e7-4810-8d10-86d4f7d0f64b

📥 Commits

Reviewing files that changed from the base of the PR and between 60f15a7 and 7e50f4b.

📒 Files selected for processing (2)

• libs/core/algorithms/include/hpx/parallel/algorithms/minmax.hpp
• libs/core/algorithms/tests/unit/algorithms/minmax_element.cpp