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

Update the internal iteration protocol #100

Closed
wants to merge 6 commits into from
Closed

Conversation

tcbrindle
Copy link
Owner

@tcbrindle tcbrindle commented Jul 14, 2023

This is an experimental PR to try out the ideas in #99, namely replacing the existing for_each_while(seq, pred) -> cursor_t customisation point with a new pair of customisation points:

auto iterate_while(auto& seq, auto pred, cursor_t from) -> cursor_t;
auto iterate_while(auto& seq, auto pred, cursor_t from, cursor_t to) -> cursor_t;

The idea being that this will allow us to use internal iteration in more places.

A rough plan of action:

  • Add flux::iterate_while wrapper functions
  • Implement flux::for_each_while using iterate_while
  • Replace existing for_each_while custom implementations with iterate_while customisations:
    • T[N]
    • std::reference_wrapper
    • ranges::contiguous_range
    • simple_sequence_base
    • chain_adaptor
    • cycle_adaptor
    • filter_adaptor
    • (multipass) flatten_adaptor
    • map_adaptor
    • passthrough_traits_base
    • reverse_adaptor
    • scan_adaptor
    • scan_first_adaptor
    • take_adaptor
    • take_while_adaptor
    • array_ptr
    • range_sequence
    • repeat_sequence
    • single_sequence
    • unfold_sequence
  • Verify that we're again getting internal iteration happening in all the places it did before
  • Update fold_first to use iterate_while
  • Implement iterate_while for subsequence
  • Add a benchmark to verify that we're now getting improved performance with the new changes
  • Add documentation for iterate_while

@tcbrindle
Copy link
Owner Author

@brycelelbach Don't worry, I'm not going to change anything out from under you: even if I get this all done and it shows an improvement, I'll wait till you've finished your work on #92 and then update cartesian_product to use the new protocol if necessary

@codecov
Copy link

codecov bot commented Jul 14, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.03% ⚠️

Comparison is base (0a0abbb) 97.67% compared to head (9901983) 97.65%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #100      +/-   ##
==========================================
- Coverage   97.67%   97.65%   -0.03%     
==========================================
  Files          66       66              
  Lines        2236     2172      -64     
==========================================
- Hits         2184     2121      -63     
+ Misses         52       51       -1     
Files Changed Coverage Δ
include/flux/op/filter.hpp 100.00% <ø> (ø)
include/flux/core/default_impls.hpp 100.00% <100.00%> (ø)
include/flux/core/sequence_access.hpp 100.00% <100.00%> (ø)
include/flux/op/for_each_while.hpp 100.00% <100.00%> (ø)
include/flux/op/map.hpp 93.33% <100.00%> (ø)

... and 12 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@brycelelbach
Copy link
Collaborator

@tcbrindle sounds good - I'll try to get you an updated PR sometime next week.

@brycelelbach
Copy link
Collaborator

@tcbrindle On the topic of improving the internal iteration protocol, I'd like to see the protocol evolved to better support parallelism in the future. I'd love to use flux to prototype a next generation parallel algorithms library.

The customization point today, for_each_while, is an early-exit for loop abstraction.

In the non-parallel case, we can implement non-early-exit algorithms like for_each in terms of for_each_while with essentially zero cost. The inverse is not true; we can't implement early-exit algorithms like find in terms of a non-early-exit for_each.

However, in parallel, implementing non-early-exit algorithms like for_each in terms of for_each_while is NOT zero cost. To parallelize, you'd have to have some sort of concurrent rollback or cancellation mechanism. On some platforms, like GPUs, it's actually more efficient to implement parallel early-exit algorithms like find by NOT actually exiting early.

Some options:

  • Have two customization points, for_each and for_each_while.
  • Add a compile time parameter to for_each_while that indicates whether or not early-exit is needed.

@tcbrindle
Copy link
Owner Author

I'd love to use flux to prototype a next generation parallel algorithms library.

That would be awesome!

Some options:

  • Have two customization points, for_each and for_each_while.
  • Add a compile time parameter to for_each_while that indicates whether or not early-exit is needed.

I guess in general we'd want some way to indicate that we want parallel execution (for all algorithms where it's possible), perhaps something like the standard execution policies? In that case we could say that for_each_while (or iterate_while), when passed a non-sequential execution policy, doesn't do early termination. Would that be sufficient?

@tcbrindle tcbrindle marked this pull request as draft July 26, 2023 12:08
This is the top-level wrapper function (or actually pair of functions) which will eventually call custom `iterate_while()` implementations if possible, or fall back to external iteration
Covers T[N], reference_wrapper<Seq> and contiguous ranges
@tcbrindle
Copy link
Owner Author

This has bitrotted, and I have better ideas for how to solve the problem

@tcbrindle tcbrindle closed this Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants