Count combinations with replacement #737
Conversation
Only two differences with combinations *without* replacement: - this `count` instead of `checked_binomial` ; - `k` can be bigger than `n`.
For a fast test: `n` and `k` are only up to 7.
|
@phimuemue What do you think? |
There was a problem hiding this comment.
Hi @Philippe-Cholet, thanks again!
Please comment how remaining_for works - I believe that you did your homework there, but I'd really appreciate if you could clear things up for the reader.
src/combinations_with_replacement.rs
Outdated
| let count = |n: usize, k: usize| checked_binomial((n + k).saturating_sub(1), k); | ||
| let k = indices.len(); | ||
| if first { | ||
| count(n, k) | ||
| } else { | ||
| indices | ||
| .iter() | ||
| .enumerate() | ||
| // TODO: Once the MSRV hits 1.37.0, we can sum options instead: | ||
| // .map(|(i, n0)| count(n - 1 - *n0, k - i)) | ||
| // .sum() | ||
| .fold(Some(0), |sum, (i, n0)| { | ||
| sum.and_then(|s| s.checked_add(count(n - 1 - *n0, k - i)?)) | ||
| }) | ||
| } |
There was a problem hiding this comment.
I believe you that this is correct, but I have a hard time comprehending this without comments. Is it the "stars and sticks" thing that leads to binomial(n+k-1, k)?
Could you comment the loop similar to what we did in the other cases?
In addition: Could n+k overflow? And if so, shouldn't we return None here instead of panicking?
There was a problem hiding this comment.
I chose the expression "stars and bars" from wikipedia, expression I did not know. I added a comment.
Good catch n+k overflowing, handled.
And I copied/updated the previous comment about the loop.
Note that `n + k` might overflow but maybe `n + k - 1` should not.
|
bors r+ Thanks @Philippe-Cholet. One thing: I'm constantly annoyed by the |
|
Build succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page. |
|
@phimuemue I was thinking it could be And with Another better alternative I guess would be our own alternative to PS: I was also thinking about merging EDIT: Because of |
@phimuemue
Maybe a comment in
remaining_forto suggest to see the big comment incombinations.rs.I have a test
combinations_with_replacement_inexact_size_hintsif you want.