Skip to content

Replaces the Poisson rejection method implementation#1560

Merged
benjamin-lieser merged 13 commits intorust-random:masterfrom
JamboChen:possion_pd
Jan 27, 2025
Merged

Replaces the Poisson rejection method implementation#1560
benjamin-lieser merged 13 commits intorust-random:masterfrom
JamboChen:possion_pd

Conversation

@JamboChen
Copy link
Contributor

@JamboChen JamboChen commented Jan 27, 2025

  • Added a CHANGELOG.md entry

Summary

As discussed in #1515, this PR replaces the implementation of poisson::RejectionMethod with a new algorithm based on the paper .

Motivation

The new implementation offers improved performance and maintains better sampling distribution, especially for extreme values of lambda (> 1e9).

Details

In terms of performance, here are the benchmarks I ran, with the current implementation as the baseline:

poisson/100             time:   [45.5242 cycles 45.6734 cycles 45.8337 cycles]
                        change: [-86.572% -86.507% -86.438%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
  2 (2.00%) low mild
  2 (2.00%) high mild
  1 (1.00%) high severe
poisson/variable        time:   [5494.6626 cycles 5508.2882 cycles 5523.2298 cycles]
                        thrpt:  [5523.2298 cycles/100 5508.2882 cycles/100 5494.6626 cycles/100]
                 change:
                        time:   [-76.728% -76.573% -76.430%] (p = 0.00 < 0.05)
                        thrpt:  [+324.27% +326.85% +329.69%]
                        Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
  1 (1.00%) low mild
  3 (3.00%) high mild
  1 (1.00%) high severe

@JamboChen
Copy link
Contributor Author

JamboChen commented Jan 27, 2025

This test fails

The sampled mean is 14.7370005, which should be within the 99% confidence interval of poisson mean, and it passes the ks test with lambda=15.

@dhardy
Copy link
Member

dhardy commented Jan 27, 2025

I expect that test is less useful than the KS test, so feel free to remove it.

This PR should also enable KS tests for larger lambda?

@JamboChen
Copy link
Contributor Author

I randomly tested between 10 and 1000. I also tested for lambda=1e9, 1e10 and it passed, larger numbers take too long (>30min) so no results yet

Copy link
Member

@benjamin-lieser benjamin-lieser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have checked the code and it does seems to match the verbal description in the paper, I really can't read fortran. I have not checked the math in the paper.

The simple avg_test can be safely ignored in my opinion.

I would add a few more values to the KS test
let parameters = [0.1, 1.0, 7.5, 45.0, 98.0, 230.0, 4567.5, 4.4541e7];

@benjamin-lieser
Copy link
Member

Can you add the changelog?

@benjamin-lieser benjamin-lieser merged commit e06370c into rust-random:master Jan 27, 2025
17 checks passed
@dhardy
Copy link
Member

dhardy commented Jan 27, 2025

Thanks @benjamin-lieser and @JamboChen!

I'll go ahead with the release.

benjamin-lieser pushed a commit to benjamin-lieser/rand that referenced this pull request Feb 5, 2025
- [x] Added a `CHANGELOG.md` entry

# Summary

As discussed in rust-random#1515, this PR replaces the implementation of
`poisson::RejectionMethod` with a new algorithm based on the [paper
](https://dl.acm.org/doi/10.1145/355993.355997).

# Motivation

The new implementation offers improved performance and maintains better
sampling distribution, especially for extreme values of lambda (> 1e9).

# Details

In terms of performance, here are the benchmarks I ran, with the current
implementation as the baseline:

```text
poisson/100             time:   [45.5242 cycles 45.6734 cycles 45.8337 cycles]
                        change: [-86.572% -86.507% -86.438%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
  2 (2.00%) low mild
  2 (2.00%) high mild
  1 (1.00%) high severe
poisson/variable        time:   [5494.6626 cycles 5508.2882 cycles 5523.2298 cycles]
                        thrpt:  [5523.2298 cycles/100 5508.2882 cycles/100 5494.6626 cycles/100]
                 change:
                        time:   [-76.728% -76.573% -76.430%] (p = 0.00 < 0.05)
                        thrpt:  [+324.27% +326.85% +329.69%]
                        Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
  1 (1.00%) low mild
  3 (3.00%) high mild
  1 (1.00%) high severe
```
benjamin-lieser pushed a commit to benjamin-lieser/rand that referenced this pull request Feb 5, 2025
- [x] Added a `CHANGELOG.md` entry

# Summary

As discussed in rust-random#1515, this PR replaces the implementation of
`poisson::RejectionMethod` with a new algorithm based on the [paper
](https://dl.acm.org/doi/10.1145/355993.355997).

# Motivation

The new implementation offers improved performance and maintains better
sampling distribution, especially for extreme values of lambda (> 1e9).

# Details

In terms of performance, here are the benchmarks I ran, with the current
implementation as the baseline:

```text
poisson/100             time:   [45.5242 cycles 45.6734 cycles 45.8337 cycles]
                        change: [-86.572% -86.507% -86.438%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
  2 (2.00%) low mild
  2 (2.00%) high mild
  1 (1.00%) high severe
poisson/variable        time:   [5494.6626 cycles 5508.2882 cycles 5523.2298 cycles]
                        thrpt:  [5523.2298 cycles/100 5508.2882 cycles/100 5494.6626 cycles/100]
                 change:
                        time:   [-76.728% -76.573% -76.430%] (p = 0.00 < 0.05)
                        thrpt:  [+324.27% +326.85% +329.69%]
                        Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
  1 (1.00%) low mild
  3 (3.00%) high mild
  1 (1.00%) high severe
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants