Added a Distribution<usize> for Poisson

[CGMossa]

I just needed this and couldn't implement it in my own crate.

Using the casting from f64 -> u64 -> usize.

I'd like to hear suggestions and concerns, as I am unsure this is the best thing to do for this type of conversation.

[CGMossa]

Hey, am I supposed to do something here or just wait for another review?

[bjorn3]

Probably just wait. I am not a member of rust-random.

[CGMossa]

Thanks!

…

On Sat, 4 Apr 2020 at 18:09, bjorn3 ***@***.***> wrote: Probably just wait. I am not a member of rust-random. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#958 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIDVSGOA3ESUML6KM5O2WTRK5LSNANCNFSM4L22JCYQ> .

[dhardy]

Thanks for the PR @CGMossa.

It seems fine, even if it is somewhat redundant functionality.

@vks?

[dhardy]

This is incorrect for 32-bit platforms. Check this out.

This isn't your fault; to_u64 for f64 makes the same mistake. Hmm, even the TryFrom in Rust 1.34 doesn't handle this conversion. I'm starting to hate this topic; seems simple but isn't.

[CGMossa]

Ohh this issue. I don't know how to solve this aswell. The link didn't suggest a solution. I think this belongs in another PR, or something?

[dhardy]

Yes: #973

[vks]

I'm a bit wary of sampling usize because of portability hazards: the results for 32 bit and 64 bit platforms may be different. However, this probably does not apply here, because it converts from floats.

What is the motivation for adding this instead of letting the user deal with the conversion from a float? Should we also add support for the other integer types?

[burdges]

Why is this useful? If you do want this then I'd suspect you want usize inside another type, in which case this works:

struct MyUsize(size);
impl Distribution<MyUsize> for Poisson { }

[CGMossa]

I just thought that this was just a little missing detail to add.
It is openning up another issue with casting f64 to usize, which is fine. If
the right way to this is to be found somewhere, I'd be happy to write it.

I don't think there should be implementation for all integer types, but for count-distributions, it
is not a stretch at all, to expect their realisations to be count-compatible types. So I still think this
is very-nice-to-have, and welcome more suggestions to improve it.

[burdges]

As a rule, implementations for usize should use the u32 implementation and panic if usize is smaller than u32.

[CGMossa]

As a rule, implementations for usize should use the u32 implementation and panic if usize is smaller than u32.

I think this does that. Do you mind checking?

[CGMossa]

I think the API should be consistent with what it is supposed to reflect. A Poisson distribution have realisations that are usize, u32, u64 and thus I think it should yield that.

I'd love to carry argue more for this, or even do more necessary work to make that happen.

[dhardy]

Actually, this seems fine as-is to me, except for the issue I raised. But maybe we should solve that separately. I'll open an issue.

[dhardy]

LGTM, aside from #973. @vks?

[vks]

This may panic on 32-bit platforms. Maybe we should document that somewhere?

[vks]

Unfortunately, this is incompatible with the choices in #987. We now require the user to perform the conversion from f64 or f32.