Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upAdd rayon example with any and all #335
Conversation
This comment has been minimized.
This comment has been minimized.
|
Outstanding example! I think the description is especially well written. Just a minor fix on a missing word. |
j-haj
reviewed
Oct 16, 2017
|
|
||
| [![rayon-badge]][rayon] [![cat-concurrency-badge]][cat-concurrency] | ||
|
|
||
| The `rayon` provides a [`ParallelIterator`] trait which includes parallelized implementations of many of the methods from the standard library's [`Iterator`] trait. |
This comment has been minimized.
This comment has been minimized.
|
|
||
| The `rayon` provides a [`ParallelIterator`] trait which includes parallelized implementations of many of the methods from the standard library's [`Iterator`] trait. | ||
|
|
||
| This example demonstrates using the [`any`] and [`all`] methods. [`any`] checks in parallel whether any element of the iterator matches the predicate, and returns as soon as one is found. [`all`] checks in parallel whether all elements of the iterator match the predicate, and returns as soon as a non-matching element is found. |
This comment has been minimized.
This comment has been minimized.
j-haj
Oct 16, 2017
Contributor
I think we should use rayon::any and rayon::all here, since there are also functions any and all in the standard library. The concern isn't so much clarity (I think this example is very well written) but more so about namespacing the function names in our links.
| use rayon::prelude::*; | ||
| fn main() { |
This comment has been minimized.
This comment has been minimized.
j-haj
Oct 16, 2017
Contributor
There seems to be a mix of examples using a main() function and a those using error_chain. I'm not sure the Contributing guide is clear on this either, as it does not explicitly state contributions should use error_chain. On the other hand, there are no errors we need to worry about here, so maybe using error_chain unnecessarily complicates the examples.
@budziq what are your thoughts here? I am somewhat partial to using a consistent approach across examples.
This comment has been minimized.
This comment has been minimized.
budziq
Oct 16, 2017
Collaborator
@j-haj Thanks for taking this on
You are right our note about error handling does not specifically state what is the correct course of action if there are not errors to handle.
The approach is as follows. If there are no errors to handle, there is no reason to increase the complexity of the example with run and error_chain and we just use plain main. We might be interested in adding this information to our contributing and error handling sections
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Thanks for the review @j-haj! |
budziq
requested changes
Oct 17, 2017
|
Thanks for the PR @CJStadler! It is almost perfect. I'd suggest only minor changes. |
|
|
||
| [![rayon-badge]][rayon] [![cat-concurrency-badge]][cat-concurrency] | ||
|
|
||
| The `rayon` crate provides a [`ParallelIterator`] trait which includes parallelized implementations of many of the methods from the standard library's [`Iterator`] trait. |
This comment has been minimized.
This comment has been minimized.
budziq
Oct 17, 2017
Collaborator
I'd suggest to omit this first paragraph as the reader should already be familiar with rayon basics via the previous examples. On the other hand we might want to mention that rayon::any and rayon::all are parallel versions of their std counterparts.
This comment has been minimized.
This comment has been minimized.
CJStadler
Oct 19, 2017
Author
Contributor
| fn main() { | ||
| let mut vec = vec![2, 4, 6, 8]; | ||
| let any_odd = vec.par_iter().any(|n| (*n % 2) != 0); |
This comment has been minimized.
This comment has been minimized.
budziq
Oct 17, 2017
Collaborator
I'd suggest to write these just as a single line assertions
assert!(!vec.par_iter().any(|n| (*n % 2) != 0));this would be slightly more concise
This comment has been minimized.
This comment has been minimized.
CJStadler
Oct 19, 2017
Author
Contributor
Personally I think it's slightly more readable to break up the line and give this a name, but I'm happy to change it.
| let all_even = vec.par_iter().all(|n| (*n % 2) == 0); | ||
| assert!(all_even); | ||
This comment has been minimized.
This comment has been minimized.
budziq
Oct 17, 2017
Collaborator
how about we add a little flavor to the second predicate like some inequality?
assert!(vec.par_iter().all(|n| *n < 10));
vec.push(13);
This comment has been minimized.
This comment has been minimized.
CJStadler
Oct 19, 2017
Author
Contributor
It seems like this loses some of the power of the example to demonstrate the relationship between the methods (when not any are odd then all are even, when any are odd then not all are even).
This comment has been minimized.
This comment has been minimized.
budziq
Oct 24, 2017
Collaborator
Imho the std examples for all/any are a little more expressive as these are showing inequality instead of just equality operators. We can always add another two asserts with different predicates
| @@ -3,6 +3,7 @@ | |||
| | Recipe | Crates | Categories | | |||
| |--------|--------|------------| | |||
| | [Mutate the elements of an array in parallel][ex-rayon-iter-mut] | [![rayon-badge]][rayon] | [![cat-concurrency-badge]][cat-concurrency] | | |||
| | [Check the elements of an array against a predicate in parallel][ex-rayon-any-all] | [![rayon-badge]][rayon] | [![cat-concurrency-badge]][cat-concurrency] | | |||
This comment has been minimized.
This comment has been minimized.
budziq
Oct 17, 2017
Collaborator
How about renaming the example to simply:
"Test in parallel if all or any collection elements match given predicate"
We are not limited to arrays here (and actually any collection/iterator viable for par_Iter will do)
This comment has been minimized.
This comment has been minimized.
| let all_even = vec.par_iter().all(|n| (*n % 2) == 0); | ||
| assert!(all_even); | ||
This comment has been minimized.
This comment has been minimized.
budziq
Oct 24, 2017
Collaborator
Imho the std examples for all/any are a little more expressive as these are showing inequality instead of just equality operators. We can always add another two asserts with different predicates
| vec.push(3); | ||
| assert!(vec.par_iter().any(|n| (*n % 2) != 0)); // At least 1 is odd. | ||
| assert!(!vec.par_iter().all(|n| (*n % 2) == 0)); // Not all are even. |
This comment has been minimized.
This comment has been minimized.
budziq
Oct 24, 2017
Collaborator
Imho the std examples for all/any are a little more expressive as these are showing inequality instead of just equality operators. We can always add another two asserts with different predicates. Please consider slightly modifying or expanding the predicates and we are ready to merge.
added some commits
Oct 15, 2017
CJStadler
force-pushed the
CJStadler:rayon-any-all
branch
from
99ae3fd
to
cfaac04
Oct 29, 2017
This comment has been minimized.
This comment has been minimized.
|
Thanks @budziq! I added some examples with inequality. |
budziq
merged commit 705475c
into
rust-lang-nursery:master
Oct 30, 2017
This comment has been minimized.
This comment has been minimized.
|
Nicely done @CJStadler ! Please also checkout #209 and comment there if you consent to repository re-license to CC0. |
CJStadler commentedOct 15, 2017
•
edited by budziq
fixes #331
edit: added
fixes #311clause to description to close the issue on merge.