Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A small performance improvement and more sensible benchmarks #2203

Merged
merged 3 commits into from
Jun 16, 2019

Conversation

pleroy
Copy link
Member

@pleroy pleroy commented Jun 15, 2019

Let's exercise only arguments that are in range...

Run on (4 X 3310 MHz CPU s)
CPU Caches:
  L1 Data 32K (x4)
  L1 Instruction 32K (x4)
  L2 Unified 262K (x4)
  L3 Unified 6291K (x1)
---------------------------------------------------------
Benchmark                  Time           CPU Iterations
---------------------------------------------------------
BM_Elbdj                 274 ns        274 ns    9976000
BM_Elbdj                 275 ns        274 ns    9976000
BM_Elbdj                 273 ns        271 ns    9976000
BM_Elbdj                 272 ns        274 ns    9976000
BM_Elbdj                 273 ns        271 ns    9976000
BM_Elbdj                 273 ns        272 ns    9976000
BM_Elbdj                 272 ns        272 ns    9976000
BM_Elbdj                 272 ns        272 ns    9976000
BM_Elbdj                 274 ns        272 ns    9976000
BM_Elbdj                 272 ns        272 ns    9976000
BM_Elbdj_mean            273 ns        272 ns         10
BM_Elbdj_median          273 ns        272 ns         10
BM_Elbdj_stddev            1 ns          1 ns         10

pleroy added 2 commits June 15, 2019 20:52

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@eggrobin eggrobin added the LGTM label Jun 16, 2019
@@ -60,7 +60,7 @@ void Elbdj(double const phi,
// TODO(phl): CHECK_EQ(π / 2, phi + phic);
double m, nc, h, c, x, d2, z, bc, dc, jc, sz, t, v, t2;

if (phi < 1.345) {
if (phi < 1.249) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we document this?
Previously the lack of documentation was explained by "translated from undocumented FORTRAN", but if we start making changes we should probably comment our magic constants.

Sorry, something went wrong.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added a big bad block of comments, but mostly this is "let's make the code do what the paper says" (the paper has like 2 pages to explain this constant).

Sorry, something went wrong.

@pleroy pleroy merged commit b0eb7b6 into mockingbirdnest:master Jun 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants