Skip to content

math: use Sincos instead of Sin and Cos in Jn and Yn #31019

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

Closed
wants to merge 1 commit into from
Closed

math: use Sincos instead of Sin and Cos in Jn and Yn #31019

wants to merge 1 commit into from

Conversation

nsajko
Copy link
Contributor

@nsajko nsajko commented Mar 25, 2019

No description provided.

@googlebot googlebot added the cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. label Mar 25, 2019
@gopherbot
Copy link
Contributor

This PR (HEAD: eb9ed7d) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/169078 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Neven Sajko:

Patch Set 1:

This was benchmarked on a Linux amd64 on a i5-8300H.

The benchmark results are strange, but I guess they can be ignored because it is clear from the diff this should be a positive change.


Please don’t reply on this GitHub thread. Visit golang.org/cl/169078.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Robert Griesemer:

Patch Set 1:

(3 comments)

LGTM but I have some suggestions.


Please don’t reply on this GitHub thread. Visit golang.org/cl/169078.
After addressing review feedback, remember to publish your drafts!

This should be slightly faster.

Benchmarks:

name  old time/op  new time/op  delta
Jn-8   108ns ± 0%   109ns ± 0%  +0.93%  (p=0.000 n=20+16)
Yn-8   106ns ± 0%   106ns ± 0%    ~     (all equal)
@gopherbot
Copy link
Contributor

This PR (HEAD: 7c3d813) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/169078 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Robert Griesemer:

Patch Set 2: Run-TryBot+1

LGTM but please only mention the relevant benchmarks in the CL description. Thanks.


Please don’t reply on this GitHub thread. Visit golang.org/cl/169078.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 2:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=29db3015


Please don’t reply on this GitHub thread. Visit golang.org/cl/169078.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 2:

Build is still in progress...
This change failed on misc-vet-vetall:
See https://storage.googleapis.com/go-build-log/29db3015/misc-vet-vetall_ec5ec67a.log

Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report.


Please don’t reply on this GitHub thread. Visit golang.org/cl/169078.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 2: TryBot-Result-1

1 of 19 TryBots failed:
Failed on misc-vet-vetall: https://storage.googleapis.com/go-build-log/29db3015/misc-vet-vetall_ec5ec67a.log

Consult https://build.golang.org/ to see whether they are new failures.


Please don’t reply on this GitHub thread. Visit golang.org/cl/169078.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Neven Sajko:

Patch Set 2:

Patch Set 2: Run-TryBot+1

LGTM but please only mention the relevant benchmarks in the CL description. Thanks.

The commit message in the Github commit after the force-push has only two benchmarks, but it seems that did not propagate to Gerrit. Should I edit the commit message directly in the Gerrit interface?


Please don’t reply on this GitHub thread. Visit golang.org/cl/169078.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Brad Fitzpatrick:

Patch Set 2:

Patch Set 2:

Patch Set 2: Run-TryBot+1

LGTM but please only mention the relevant benchmarks in the CL description. Thanks.

The commit message in the Github commit after the force-push has only two benchmarks, but it seems that did not propagate to Gerrit. Should I edit the commit message directly in the Gerrit interface?

No. See https://github.com/golang/go/wiki/CommitMessage#github-pull-requests


Please don’t reply on this GitHub thread. Visit golang.org/cl/169078.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Neven Sajko:

Patch Set 3:

Patch Set 2:

Patch Set 2:

Patch Set 2: Run-TryBot+1

LGTM but please only mention the relevant benchmarks in the CL description. Thanks.

The commit message in the Github commit after the force-push has only two benchmarks, but it seems that did not propagate to Gerrit. Should I edit the commit message directly in the Gerrit interface?

No. See https://github.com/golang/go/wiki/CommitMessage#github-pull-requests

OK, sorry about that. It should be OK now.


Please don’t reply on this GitHub thread. Visit golang.org/cl/169078.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Robert Griesemer:

Patch Set 3:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/169078.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Neven Sajko:

Patch Set 3:

(1 comment)

Patch Set 3:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/169078.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Robert Griesemer:

Patch Set 3:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/169078.
After addressing review feedback, remember to publish your drafts!

gopherbot pushed a commit that referenced this pull request Mar 25, 2019
Change-Id: I0da3857013f1d4e90820fb043314d78924113a27
GitHub-Last-Rev: 7c3d813
GitHub-Pull-Request: #31019
Reviewed-on: https://go-review.googlesource.com/c/go/+/169078
Reviewed-by: Robert Griesemer <[email protected]>
@gopherbot
Copy link
Contributor

Message from Robert Griesemer:

Patch Set 4: Code-Review+2

LGTM. Thanks!


Please don’t reply on this GitHub thread. Visit golang.org/cl/169078.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR is being closed because golang.org/cl/169078 has been merged.

@gopherbot gopherbot closed this Mar 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants