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

Group by async_apply #220

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

akleeman
Copy link
Collaborator

This adds the async_apply methods which work on map-like objects which makes it compatible with group_by.

@akleeman akleeman force-pushed the group_by_async_apply branch from 86a9d2b to e67b895 Compare March 27, 2020 22:06
@martin-swift
Copy link
Contributor

martin-swift commented Mar 27, 2020

from slack - I applied this on top of the no-copy branch (rebased on top of the async utils) and there was a little problem; the lambda I was using for the mean won't compile anymore:

    auto predict_single_mean = [&](const auto &f) {
      return iono_model_->predict_mean({f})[0];
    };

fails with this error:

/home/stiaszny/tmp/orion/third_party/orion-engine/src/ssr_generator/ssr_generator.cc:307:44: error: converting to 'const std::vector<orion::SignalArc, std::allocator<orion::SignalArc> >'
 from initializer list would use explicit constructor 'std::vector<_Tp, _Alloc>::vector(const allocator_type&) [with _Tp = orion::SignalArc; _Alloc = std::allocator<orion::SignalArc>; std::vector<_Tp, _Alloc>::allocator_type = std::allocator<orion::SignalArc>]'
       return iono_model_->predict_mean({f})[0];

reverting this change (and my one include) makes the lambda compile again.

@akleeman akleeman force-pushed the group_by_async_apply branch from e67b895 to 511018d Compare March 28, 2020 00:40
@akleeman
Copy link
Collaborator Author

Try again, I think I might have fixed the issue. Basically if you do:

std::vector<ValueType> values;
for (const auto &v : values) { ... }

auto might have been resolving to std::allocator<ValueType>, I explicitly made it ValueType in the async for loops.

@akleeman akleeman force-pushed the group_by_async_apply branch from 511018d to 978fcf7 Compare March 28, 2020 00:46
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