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

Implement Jacobi amplitude #2233

Merged
merged 10 commits into from
Jul 4, 2019
Merged

Implement Jacobi amplitude #2233

merged 10 commits into from
Jul 4, 2019

Conversation

pleroy
Copy link
Member

@pleroy pleroy commented Jun 30, 2019

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_JacobiAmplitude               186 ns        184 ns   15090000
BM_JacobiAmplitude               194 ns        189 ns   15090000
BM_JacobiAmplitude               185 ns        186 ns   15090000
BM_JacobiAmplitude               185 ns        184 ns   15090000
BM_JacobiAmplitude               186 ns        184 ns   15090000
BM_JacobiAmplitude               186 ns        186 ns   15090000
BM_JacobiAmplitude               184 ns        185 ns   15090000
BM_JacobiAmplitude               185 ns        183 ns   15090000
BM_JacobiAmplitude               185 ns        183 ns   15090000
BM_JacobiAmplitude               184 ns        185 ns   15090000
BM_JacobiAmplitude_mean          186 ns        185 ns         10
BM_JacobiAmplitude_median        185 ns        185 ns         10
BM_JacobiAmplitude_stddev          3 ns          2 ns         10
BM_JacobiSNCNDN                  131 ns        131 ns   21890000
BM_JacobiSNCNDN                  133 ns        131 ns   21890000
BM_JacobiSNCNDN                  132 ns        132 ns   21890000
BM_JacobiSNCNDN                  132 ns        132 ns   21890000
BM_JacobiSNCNDN                  132 ns        130 ns   21890000
BM_JacobiSNCNDN                  132 ns        131 ns   21890000
BM_JacobiSNCNDN                  131 ns        132 ns   21890000
BM_JacobiSNCNDN                  131 ns        130 ns   21890000
BM_JacobiSNCNDN                  131 ns        131 ns   21890000
BM_JacobiSNCNDN                  134 ns        135 ns   21890000
BM_JacobiSNCNDN_mean             132 ns        131 ns         10
BM_JacobiSNCNDN_median           132 ns        131 ns         10
BM_JacobiSNCNDN_stddev             1 ns          1 ns         10

} else {
double const k = EllipticK(mc);
JacobiSNCNDNWithK(u, mc, k, s, c, d);
offset = 2.0 * π * std::round(u / (4.0 * k)) * Radian;
Copy link
Member

Choose a reason for hiding this comment

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

why not std::nearbyint?

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.

Done.

Sorry, something went wrong.

@eggrobin eggrobin added the LGTM label Jun 30, 2019
@pleroy
Copy link
Member Author

pleroy commented Jul 3, 2019

PTAL, I added a monotonicity test and fixed the implementation. New benchmark:

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_JacobiAmplitude               199 ns        200 ns   14360000
BM_JacobiAmplitude               200 ns        199 ns   14360000
BM_JacobiAmplitude               200 ns        198 ns   14360000
BM_JacobiAmplitude               199 ns        199 ns   14360000
BM_JacobiAmplitude               199 ns        198 ns   14360000
BM_JacobiAmplitude               199 ns        198 ns   14360000
BM_JacobiAmplitude               199 ns        199 ns   14360000
BM_JacobiAmplitude               198 ns        196 ns   14360000
BM_JacobiAmplitude               199 ns        197 ns   14360000
BM_JacobiAmplitude               199 ns        200 ns   14360000
BM_JacobiAmplitude_mean          199 ns        198 ns         10
BM_JacobiAmplitude_median        199 ns        198 ns         10
BM_JacobiAmplitude_stddev          1 ns          1 ns         10

@pleroy
Copy link
Member Author

pleroy commented Jul 3, 2019

retest this please

@pleroy pleroy merged commit 9efffef into mockingbirdnest:master Jul 4, 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