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

pared down mod2pi pull request #4862

Merged
merged 13 commits into from
Dec 16, 2013
Merged

pared down mod2pi pull request #4862

merged 13 commits into from
Dec 16, 2013

Conversation

StefanKarpinski
Copy link
Sponsor Member

This is a counterpoint pull request, building on the excellent work of #4799. It's basically purely editorial – I just removed (and moved) things. For a first pass, adding three functions to Base seems like far too much. I decided to keep mod2pi of the three originally introduced functions since it's the most sensible one, modding down to a single turn of the unit circle. I'm really beginning to agree with the "pi is wrong" sentiment – if we used τ instead of π then modtau would be the unequivocal right choice.

@StefanKarpinski
Copy link
Sponsor Member Author

So one issue is that there does seem to be a limit to the range in which this is accurate:

julia> mod2pi(1e100)
5.892699463987461

Wolfram|Alpha says the correct value is 3.52316061552557...

@StefanKarpinski
Copy link
Sponsor Member Author

cc: @fabianlischka

@fabianlischka
Copy link
Contributor

Hi Stefan, well actually (sorry), 1e100 is a Float64, and it is not 10^100 (which is what WolframAlpha interprets it as), but rather (1+643957962097533/2^52)*2^332 or 10000000000000000159028911097599180468360808563945281389781327557747838772170381060813469985856815104.
That, in turn, mod 2pi is indeed 5.8926994639874608774... according to the Gospel of Alpha.

It might not be what the user intended, but it's hard to catch, because 1e100 comes in as a Float64.
I was hoping that mod2pi(10^100) complains, but 10^100 is 0 after overflow. So, I think we'll have to live with this.

@fabianlischka
Copy link
Contributor

BTW, looks like you took out modpi and modpio2 (sniff...), but left them in the documentation (both files).

@fabianlischka
Copy link
Contributor

The gaps between consecutive Float64 (while all integers) are rather severe there:
julia> eps(1e100)
1.942668892225729e84

:-)

mod2pi becomes pretty meaningless there (though surprisingly accurate)

@StefanKarpinski
Copy link
Sponsor Member Author

Right. I wonder if we should check for it and throw an error or what.

@fabianlischka
Copy link
Contributor

Good question, but I think we should NOT throw an error:

a) where do you cut off? We know that from about 2^53 we can't represent every integer as a Float64, but maybe the user typed in an int that can be represented. Or maybe he typed in 0.5 plus that, and it got rounded. mod2pi() will return the result for the Float64 passed in, not for the decimal representation that the user typed in. We know that eps increases gradually. The Float64 could be off by eps/2 from the decimal the user typed in, so mod2pi could be off by eps/2 from the number the user intended. At which point do we cut off?

This is a fundamental property of IEEE754 Float64s, not of the mod2pi function.

b) Compare:
julia> sin(1e100)
-0.3806377310050287

Alpha: sin(1e100)
-0.98480775301220805936674302458952301367064325171984241879002...

But:
Alpha: sin(10000000000000000159028911097599180468360808563945281389781327557747838772170381060813469985856815104) # that's what 1e100 as a Float64 actually is
-0.38063773100502866607187092333210730322126753816281538528564...
lining up with mod2pi perfectly.

And we don't throw an error for sin(x) for large x.

@StefanKarpinski
Copy link
Sponsor Member Author

Ah, yes. You are right, of course. I was forgetting that w|α was interpreting my 1e100 as an exact decimal value.

@kmsquire
Copy link
Member

+1 for adding a definition for τ to Julia. (I was meaning to propose this right before Tau Day this year, but just missed it...)

@simonbyrne
Copy link
Contributor

Distributions.jl defines twoπ as a constant, we could import that into Base?

@StefanKarpinski
Copy link
Sponsor Member Author

Perhaps we should just have tau as a MathConst instead.

@kmsquire
Copy link
Member

+1 (am I allowed more than one vote? :-)

On Wednesday, November 20, 2013, Stefan Karpinski wrote:

Perhaps we should just have tau as a MathConst instead.


Reply to this email directly or view it on GitHubhttps://github.com//pull/4862#issuecomment-28897675
.

@ViralBShah
Copy link
Member

Bump.

@StefanKarpinski
Copy link
Sponsor Member Author

If you want to merge and then remove the extra docs for functions I deleted, go for it. I'm going to speak at Code Mesh in the morning, so I'm not going to mess with it now.

StefanKarpinski added a commit that referenced this pull request Dec 16, 2013
pared down mod2pi pull request
@StefanKarpinski StefanKarpinski merged commit 48e8037 into master Dec 16, 2013
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.

None yet

5 participants