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

RFC: Implement access::components higher-order range mapper #193

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fknorr
Copy link
Contributor

@fknorr fknorr commented Jul 13, 2023

RFC, based on #182.

A common parallelization pattern, e.g. in dense matrix-vector products, is to map 1D thread-ids to the rows of a 2D matrix while iterating over all columns on each item. This currently requires a custom range mapper:

const auto rows = [](const chunk<1> &chnk, const range<2> &buffer_size) {
    return subrange<2>{
        {chnk.offset[0], 0},
        {chnk.range[0], buffer_size[1]}
    };
};

While we could just have access::rows and access::columns convenience range mappers with all the confusion about row-major vs column-major vs fastest-dimensions that would entail, I have come up with a generic solution for the entire class of these "component mappings".

Let me introduce the first higher-order range mapper, access::components. It constructs a (chunk<KernelDims>, range<BufferDims>) -> subrange<BufferDims> range mapper from BufferDims individual mappers of type (chunk<KernelDim>, range<1>) -> subrange<1>.

Together with the new, straight-forward mapper access::kernel_dim that creates a subrange<1> from a single kernel dimension, this allows us to re-write the custom range mapper above like so:

const auto rows = access::components(access::kernel_dim(0), access::all());

Note that any other range mapper that produces subrange<1> can be used for each component, such as fixed or neighborhood.

@fknorr fknorr requested review from psalz and PeterTh July 13, 2023 16:10
@fknorr fknorr self-assigned this Jul 13, 2023
@fknorr fknorr changed the base branch from docs-overview-update to master July 13, 2023 16:10
@fknorr fknorr changed the title RFC: Implement access::components higher-order range mapper RFC: Implement access::components range mapper combinator Jul 13, 2023
Copy link
Contributor

@PeterTh PeterTh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. I'm not sure about the kernel_dim name, but I do like components.

test/runtime_tests.cc Outdated Show resolved Hide resolved
test/runtime_tests.cc Outdated Show resolved Hide resolved
@github-actions
Copy link

Check-perf-impact results: (9e900a306dc9f17e4a27439205a7680c)

❓ No new benchmark data submitted. ❓
Please re-run the microbenchmarks and include the results if your commit could potentially affect performance.

@fknorr fknorr changed the title RFC: Implement access::components range mapper combinator RFC: Implement access::components higher-order range mapper Jul 13, 2023
@fknorr
Copy link
Contributor Author

fknorr commented Jul 13, 2023

Live bikeshedding update about the naming of access::kernel_dim:

  • All other built-in range mapperns describe a pattern while kernel_dim describes a function. That works within the components "DSL" but looks out-of-place as a stand-alone RM.
  • @PeterTh suggested we could extend one_to_one to optionally take a dim parameter and then act as chunk<Dims> -> subrange<1>, because on a high level we think of the rows RM from the OP to act like a one_to_one in the first dimension and like an all in the second one. However this breaks again when regarding this specialization as a standalone RM, where a chunk<2> -> subrange<1> with that definition is an n-to-one, not a one-to-one.
  • Naming proposals: @fknorr access::linear, access::line or access::interval.

More shower thoughts from my side:

  • Since this must inspect the RM result subranges for each component, it might interfere with our goal of allowing RMs to return regions in the future.
  • neighborhood unfortunately cannot be used in a component since input and output dimensions must match.
  • Although "higher order range mappers" feel very neat conceptually, the few relevant component mappings (kernel_dim, fixed, all) could also be expressed through purpose-built tag types that lead to a less surprising interface.

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