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

Adds parallel map-reduce recipe #342

Merged
merged 1 commit into from Oct 27, 2017

Conversation

Projects
None yet
2 participants
@j-haj
Copy link
Contributor

j-haj commented Oct 24, 2017

fixes #329

  • Adds example in concurrency.md
  • Adds links to intro.md
@budziq
Copy link
Collaborator

budziq left a comment

Very cool. Just few nits below

let num_over_30 = v.par_iter().filter(|&ref x| gt_30(x.age)).count();
let sum_over_30 = v.par_iter()
.filter(|&ref x| gt_30(x.age))
.map(|ref x| x.age)

This comment has been minimized.

@budziq

budziq Oct 25, 2017

Collaborator

I'd suggest mapping first and then using filter(gt_30) instead of closure in closure

.reduce(|| 0, |x, y| x + y);
// Rayon also has a sum() method, which yields the same results as the
// reduction on the previous line

This comment has been minimized.

@budziq

budziq Oct 25, 2017

Collaborator

I'd move this comment to the textual description above

let avg_over_30 = sum_over_30 as f32 / num_over_30 as f32;
let alt_avg_over_30 = alt_sum_30 as f32 / num_over_30 as f32;
assert!((avg_over_30 - alt_avg_over_30).abs() < 1e-8);

This comment has been minimized.

@budziq

budziq Oct 25, 2017

Collaborator

I'd suggest using std::f32::EPSILON instead of 1e-8

.map(|ref x| x.age)
.sum();
let avg_over_30 = sum_over_30 as f32 / num_over_30 as f32;

This comment has been minimized.

@budziq

budziq Oct 25, 2017

Collaborator

I'd probably move casts to above and store the variables as f32 but it's a matter of taste.

This comment has been minimized.

@j-haj

j-haj Oct 25, 2017

Author Contributor

I run into a type inference issue with the alt_sum_30 variable if I moved as f32 to the line where alt_sum_30 is initialized. So far the easiest way to get rid of that error is to make alt_sum_30 a u32 and then convert to f32 later. Any suggestions?

This comment has been minimized.

@budziq

budziq Oct 25, 2017

Collaborator

right sum is a template so type inference gets wonky here.

 let alt_sum_30: f32 = v.par_iter()
        .filter(|x| gt_30(x.age))
        .map(|x| x.age as f32)
        .sum();

or

    let alt_sum_30 = v.par_iter()
        .filter(|x| gt_30(x.age))
        .map(|x| x.age as f32)
        .sum::<f32>();

or the cleanest solution, leaving is as it was, summed into u32 and casting later on ;)

];
let gt_30 = |x: u32| x > 30;
let num_over_30 = v.par_iter().filter(|&ref x| gt_30(x.age)).count();

This comment has been minimized.

@budziq

budziq Oct 25, 2017

Collaborator

the &ref and ref are not needed anywhere in this example

@j-haj j-haj force-pushed the j-haj:rayon-map-reduce branch from 3f42a1e to ceca806 Oct 26, 2017

Adds parallel map-reduce recipe
See #329

* Adds example in `concurrency.md`
* Adds links to `intro.md`

@j-haj j-haj force-pushed the j-haj:rayon-map-reduce branch from ceca806 to 47c43c7 Oct 26, 2017

@j-haj

This comment has been minimized.

Copy link
Contributor Author

j-haj commented Oct 26, 2017

Thanks for all the comments! I have pushed an updated version

@budziq budziq merged commit 477d82f into rust-lang-nursery:master Oct 27, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@budziq

This comment has been minimized.

Copy link
Collaborator

budziq commented Oct 27, 2017

Nicely done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.