Add examples to the ParallelIterator documentation #467

Kerollmops · Nov 5, 2017

The last remaining examples to do are map_with and fold_with that I don't really understand.

Could you please explain to me the mecanics ?

cuviper · Nov 6, 2017

Thanks for the examples! However, this is surely going to collide in a big way with #466, which I just approved since it came in first. When that lands, please do compare and merge. Where your examples might show some distinctly interesting case, feel free to keep both, otherwise we should keep whichever example is most easily understood.

The CI failure should be fixed by #468.

cuviper · Nov 6, 2017

As for map_with, for_each_with, and fold_with, the idea is these can be used to pass around some supplemental data type which implements Clone and Send, but not necessarily Sync where it could just be shared directly.

The primary motivating example of this is the mpsc::Sender with Clone and Send but !Sync. Using these _with methods, you can get a clone of the sender in each thread as-needed.

There aren't many options to do this sort of thing manually -- e.g. maybe a Mutex<Sender> that you lock and clone in the fold() identity function, but this is kind of ugly.

Kerollmops · Nov 9, 2017

Thanks for the indications. I have rebased my branch and done some improvements on the previously merged examples.

I will improve some of the examples already present here and add examples to the map_with, for_each_with, and fold_with methods, so please don't merge it for now.

Thank you for the details about theses functions.

Kerollmops · Nov 9, 2017

I have just added an example to the for_each_with method. map_with and fold_with doesn't need much more example and like you said the Sender method is kind of ugly.

You can review this PR and merge it if you think it's good 😄

cuviper · Nov 13, 2017

cuviper requested changes Nov 13, 2017

View changes

Mostly good, but a couple of the examples are depending on sequential order. Even if these happen to work in the rustdoc+CI environment, it's not a good example for documentation.

cuviper · Nov 13, 2017

src/iter/mod.rs

+    ///
+    /// let res: Vec<_> = receiver.iter().collect();
+    ///
+    /// assert_eq!(&res[..], &[0, 1, 2, 3, 4])


I think you got lucky that this succeeded in CI, because the order of calling send in parallel is not guaranteed! Perhaps we should explicitly sort the res before comparing it.

Ho ! Yes you are right...

cuviper · Nov 13, 2017

src/iter/mod.rs

-    ///            .reduce(|| (0, 0), // the "identity" is 0 in both columns
+    ///            .par_iter()
+    ///            .cloned()
+    ///            .reduce(|| (0, 0),


Why did you remove these comments? I disagree with your commit calling this less readable -- it's trying to explicitly explain the steps that go into a reduce.

cuviper · Nov 13, 2017

src/iter/mod.rs

    /// let sum = bytes.into_par_iter()
    ///                .fold(|| 0_u32, |a: u32, b: u8| a + (b as u32))
    ///                .sum::<u32>();
-    /// assert_eq!(sum, (0..22).sum()); // compare to sequential


Again, let's leave existing comments in place.

cuviper · Nov 13, 2017

src/iter/mod.rs

    /// ```
    /// use rayon::prelude::*;
-    /// fn factorial(n: u32) -> u32 {
+    ///
+    /// fn factorial(n: usize) -> usize {


Why did you change the types? Idiomatic Rust is supposed to use explicitly-sized integers when you're not dealing with indexing or container sizes.

cuviper · Nov 13, 2017

src/iter/mod.rs

+    ///
+    /// let sum: usize = a.par_iter().cloned().while_some().sum::<usize>();
+    ///
+    /// assert_eq!(sum, 7);


I think this test is getting lucky too, but perhaps we need to clarify the documented behavior. It halts as soon as a None is found anywhere, in arbitrary parallel order. You can't assume that it will see all of the sequentially-prior Some values before finding a None, and it may also visit Some values that are sequentially after a None. In the extreme, this while_some could produce nothing if it hit the None first, or it may even produce all of the Some items if it hit the None last!

So what kind of example can I use here ? It's some sort of random that occurs...

Perhaps while_some().count() < a.len() is a better example? It's somewhat random which Some elements will make it through, but we can guarantee that it will be less than the total vector length.

You can make the same assertion about length with filter_map though - I'm not sure how to clearly demonstrate that while_some short-circuits. Maybe we could show an infinite source iterator getting stopped, like rayon::iter::repeat(None).while_some().count() == 0. Or for another example, check_while_some uses an atomic to count how many values were produced behind the scenes.

As a practical use-case, FromParallelIterator for Option and for Result each use while_some() to short-circuit their collection when they see a None or Err.

Ok, so I will use the test code, it is clear and short.

Thank you for the explaination, really usefull and clear too.
Powerfull while_some method !

cuviper · Nov 14, 2017

cuviper approved these changes Nov 14, 2017

View changes

cuviper · Nov 14, 2017

Looks great, thanks!

bors r+

bors · Nov 15, 2017

Build failed

continuous-integration/travis-ci/push

cuviper · Nov 15, 2017

error: unable to get packages from source

Caused by:

[18] Transferred a partial file (transfer closed with 897629 bytes remaining to read)

Looks spurious.

bors r+

bors · Nov 15, 2017

Build succeeded

cuviper changed the title ~~Add examples to the ParrallelIterator documentation~~ Add examples to the ParallelIterator documentation Nov 6, 2017

bors bot merged commit ebe1e28 into rayon-rs:master Nov 15, 2017
2 checks passed

2 checks passed

bors Build succeeded

continuous-integration/travis-ci/pr The Travis CI build passed
Details

rayon-rs/rayon

Join GitHub today

Add examples to the ParallelIterator documentation #467

Conversation

Kerollmops commented Nov 5, 2017 • edited

This comment has been minimized.

cuviper commented Nov 6, 2017

cuviper changed the title Add examples to the ParrallelIterator documentation Add examples to the ParallelIterator documentation Nov 6, 2017

This comment has been minimized.

cuviper commented Nov 6, 2017

Kerollmops added some commits Nov 4, 2017

Kerollmops added some commits Nov 9, 2017

This comment has been minimized.

Kerollmops commented Nov 9, 2017 • edited

Kerollmops added some commits Nov 9, 2017

This comment has been minimized.

Kerollmops commented Nov 9, 2017 • edited

cuviper requested changes Nov 13, 2017 View changes

cuviper left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Kerollmops Nov 13, 2017 • edited

This comment has been minimized.

This comment has been minimized.

Kerollmops added some commits Nov 13, 2017

cuviper approved these changes Nov 14, 2017 View changes

This comment has been minimized.

cuviper commented Nov 14, 2017

bors bot added a commit that referenced this pull request Nov 14, 2017

This comment has been minimized.

bors bot commented Nov 15, 2017

Build failed

This comment has been minimized.

cuviper commented Nov 15, 2017

bors bot added a commit that referenced this pull request Nov 15, 2017

This comment has been minimized.

bors bot commented Nov 15, 2017

Build succeeded

Hide details View details bors bot merged commit ebe1e28 into rayon-rs:master Nov 15, 2017 2 checks passed

2 checks passed

Kerollmops commented Nov 5, 2017 •

edited

Kerollmops commented Nov 9, 2017 •

edited

Kerollmops commented Nov 9, 2017 •

edited

cuviper requested changes Nov 13, 2017

View changes

Kerollmops Nov 13, 2017 •

edited

cuviper approved these changes Nov 14, 2017

View changes

bors bot merged commit `ebe1e28` into rayon-rs:master Nov 15, 2017
2 checks passed