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

sinc(π) not defined #7994

Closed
EthanAnderes opened this issue Aug 13, 2014 · 14 comments
Closed

sinc(π) not defined #7994

EthanAnderes opened this issue Aug 13, 2014 · 14 comments
Labels
maths Mathematical functions

Comments

@EthanAnderes
Copy link

The function sinc doesn't seem to be defined for π.

julia> sinc(3.1415926535897)
-0.04359862862916225

julia> sinc(π)
ERROR: `decompose` has no method matching decompose(::MathConst{:π})
 in sinpi at special/trig.jl:2
 in sinc at special/trig.jl:88

julia> sin(π)
1.2246467991473532e-16
@JeffBezanson
Copy link
Member

See also #5561. It's not clear where to stop adding definitions for MathConsts.

@StefanKarpinski
Copy link
Member

Yikes. Decompose should not really be called on MathConsts since they're typically not rational.

@tknopp
Copy link
Contributor

tknopp commented Aug 15, 2014

There are of course simple workarounds for this by writing e.g. sinc(1π).

But still I do not understand why the decompose method is called here.

@simonbyrne
Copy link
Contributor

I would never have guessed that this was the problem:

julia> isinf(pi)
ERROR: `decompose` has no method matching decompose(::MathConst{:π})
 in isinf at float.jl:217

@EthanAnderes Note that the sinc function is the normalised version that multiplies by pi internally, i.e. sinc(x) = sin(pi*x)/(pi*x).

@tknopp
Copy link
Contributor

tknopp commented Aug 15, 2014

Maybe the concept behind MathConst is a little bit to clever? Lets have a look how we do with another important constant 1:

  • Typing 1 gives an Int, typing 1.0 gives a Float64
  • If one wants this to be parametric one has to use the one method.
    If I understand the MathConst concept right one wants to delegate the conversion to the next operation happening.

Maybe one could have pi beeing a Float64 and pi{T} beeing used if one wants a specific type (this is just brainstorm not a concrete proposal).

@EthanAnderes
Copy link
Author

@simonbyrne That did initially trip me up. In fact, I was testing sinc(\pi) after seeing my code had a misspecified phase, ... then ran into it not being defined. Obviously, not a big deal but I figured someone would want to know about it. Hence the issue here. Thanks for looking into it. Cheers.

@stevengj
Copy link
Member

Regardless, I feel like isfinite should work on a mathconst, maybe via isfinite(x::MathConst) = isfinite(convert(Float64,x)). That's the sort of basic numerical test that should be available for any real numeric type.

(Although sinc still won't work because of the oftype call in sinc. But sinpi will now work.)

@kshyatt
Copy link
Contributor

kshyatt commented Sep 15, 2016

So, on latest master...

julia> sinc(π)
ERROR: MethodError: Cannot `convert` an object of type Int64 to an object of type Irrational{}
This may have arisen from a call to the constructor Irrational{:π}(...),
since type constructors fall back to convert methods.
 in sinpi(::Irrational{:π}) at ./special/trig.jl:159
 in sinc(::Irrational{:π}) at ./special/trig.jl:296

julia> sinc(3.1415926535897)
-0.04359862862916225

@kshyatt kshyatt added the maths Mathematical functions label Sep 15, 2016
@simonbyrne
Copy link
Contributor

It appears to be due to this:

julia> one(typeof(pi))
ERROR: MethodError: Cannot `convert` an object of type Int64 to an object of type Irrational{:π}
This may have arisen from a call to the constructor Irrational{:π}(...),
since type constructors fall back to convert methods.
 in one(::Type{Irrational{:π}}) at ./number.jl:64

@stevengj
Copy link
Member

It seems reasonable to define convert{T<:Irrational}(::Type{T}, x::Number) = x.

@simonbyrne
Copy link
Contributor

simonbyrne commented Sep 15, 2016

I don't know: defining convert{T}(::Type{T}, x) to give something that isn't of type T seems dangerous.

@StefanKarpinski
Copy link
Member

Hmm. This is a pretty tricky issue. We really want one(pi) to give a non-Irrational.

@martinholters
Copy link
Member

According to ?one, one(π) should give "the multiplicative identity element for the type of" π. I don't think we have that at the moment (1*π == 1.0*π ≠ π). That would leave introducing Irrational{1} (or maybe a more appropriately named singleton type, say One) with correspondingly overloaded *. I'm not sure we want to go down that road, though.

@fredrikekre
Copy link
Member

#21781

julia> sinc(π)
-0.04359862862918775

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maths Mathematical functions
Projects
None yet
Development

No branches or pull requests

10 participants