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

Usability of TExecutor::MapReduce #7871

Closed
4 tasks
hageboeck opened this issue Apr 14, 2021 · 2 comments
Closed
4 tasks

Usability of TExecutor::MapReduce #7871

hageboeck opened this issue Apr 14, 2021 · 2 comments
Assignees
Labels
fixathon This issue can be tackled at a ROOT fixathon improvement in:Documentation

Comments

@hageboeck
Copy link
Member

hageboeck commented Apr 14, 2021

Explain what you would like to see improved

It's hard to use TExecutor::MapReduce correctly, because the required signature of the reducer isn't mentioned in the documentation.

Optional: share how it could be improved

  • Add some info to all docs, e.g. that a lambda like this would do:
[](const std::vector<T>&) { }

Failure to meet that signature leads to an explosion of template instantiation failures with gcc.

To Reproduce

void runParallel() {
  ROOT::Internal::TExecutor ex;
  const auto nChunk = ROOT::IsImplicitMTEnabled() ? ex.GetPoolSize() : 1u; 
  // Empty reducer
  auto reducer = []() -> int { return 0; };
  auto printRange = [=](std::size_t) -> int {
    return 0;
  };
  
  ex.MapReduce(printRange, ROOT::TSeq<std::size_t>(0, nChunk), reducer, nChunk);
}
@eguiraud
Copy link
Member

Failure to meet that signature leads to an explosion of template instantiation failures with gcc.

We should have better SFINAE code that provides clearer errors

Show how to do a parallel for using Map()?

I think Foreach should be used for that, see my comment at #7872

@eguiraud eguiraud removed this from Needs triage in Triage Apr 15, 2021
@eguiraud eguiraud self-assigned this Apr 15, 2021
@vepadulano vepadulano added the fixathon This issue can be tackled at a ROOT fixathon label Feb 5, 2024
@peremato peremato self-assigned this Feb 14, 2024
peremato added a commit to peremato/root that referenced this issue Feb 14, 2024
peremato added a commit to peremato/root that referenced this issue Feb 14, 2024
@peremato
Copy link
Member

Adding an static_assert helps a but on the complier error report but it does not stop from trying different instantiations.

In module 'Imt':
/Users/mato/Development/root/builddir/include/ROOT/TExecutor.hxx:240:4: error: static assertion failed due to requirement 'std::is_invocable_v<(lambda at /Users/mato/Development/root/reproducer2.C:5:18), std::vector<int, std::allocator<int> > >': redfunc does not have the correct signature
   static_assert(std::is_invocable_v<R, std::vector<InvokeResult_t<F,INTEGER>>>, "redfunc does not have the correct signature");
   ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/mato/Development/root/reproducer2.C:12:15: note: in instantiation of function template specialization 'ROOT::Internal::TExecutor::MapReduce<(lambda at /Users/mato/Development/root/reproducer2.C:8:21), unsigned long, (lambda at /Users/mato/Development/root/reproducer2.C:5:18), void>' requested here
  auto n = ex.MapReduce(printRange,  ROOT::TSeq<std::size_t>(0, nChunk), reducer, nChunk);
              ^
In module 'Imt':
/Users/mato/Development/root/builddir/include/ROOT/TExecutor.hxx:244:11: error: no matching member function for call to 'Reduce'
   return Reduce(Map(func, args), redfunc);
          ^~~~~~
/Users/mato/Development/root/builddir/include/ROOT/TExecutorCRTP.hxx:151:36: note: candidate template ignored: substitution failure [with T = int, R = (lambda at /Users/mato/Development/root/reproducer2.C:5:18)]: no matching function for call to object of type '(lambda at /Users/mato/Development/root/reproducer2.C:5:18)'
   template<class T, class R> auto Reduce(const std::vector<T> &objs, R redfunc) -> decltype(redfunc(objs));
                                   ^                                                         ~~~~~~~
/Users/mato/Development/root/builddir/include/ROOT/TExecutorCRTP.hxx:150:25: note: candidate function not viable: requires single argument 'mergeObjs', but 2 arguments were provided
   template<class T> T* Reduce(const std::vector<T*> &mergeObjs);
...

dpiparo pushed a commit that referenced this issue Feb 23, 2024
@dpiparo dpiparo added this to Issues in Fixed in 6.32.00 via automation Feb 23, 2024
@dpiparo dpiparo closed this as completed Feb 23, 2024
gwmyers pushed a commit to gwmyers/root that referenced this issue Feb 23, 2024
kristupaspranc pushed a commit to kristupaspranc/root that referenced this issue Apr 10, 2024
lobis pushed a commit to lobis/root that referenced this issue Apr 10, 2024
kristupaspranc pushed a commit to kristupaspranc/root that referenced this issue May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixathon This issue can be tackled at a ROOT fixathon improvement in:Documentation
Projects
Status: Done
Development

No branches or pull requests

5 participants