doctest that only integer +/- 1 gets special treatment

[was]

This is just a doctest of a fix already in main Sage.

Original description:

This is really bad

sage: 1.0 * pi
pi
sage: sin(1.0 * pi)
0

This is good:

sage: maxima('sin(%pi*1.0)')
sin(1.0*%pi)
sage: maxima('1.0*%pi')
1.0*%pi

This is likely the fault of ginac doing something stupid with 1.0 == 1.

Ken Ribet pointed this out during the JMM 2012 meeting.

[burcin]

There are two sides to this. Doing the multiplication and printing the coefficient.

GiNaC takes a shortcut when the coefficient is 1 and skips multiplication. There is a Python callback function (sage.symbolics.pynac.py_get_parent_char) to check the characteristic of the parent. Pynac forces the operation for positive characteristic. If we return a positive integer from that function when the parent is inexact, the multiplication will go through.

If a coefficient is 1, we don't print it. So the printing code in pynac needs to be changed to print inexact coefficients. This is now ​pynac issue #4.

Even when all these are sorted out, sin(1.0*pi) might still return 0. Since bool(1.0*pi == pi) will probably be True.

[was]

It should be that sin(1.0*pi) returns 0.0. We can fix this somehow. I'm not worried about printing.

[kcrisman]

Here is another idea. In ​the Ginac source, we have

00464     } else if (seq_size==1 && overall_coeff.is_equal(_ex1)) {
00465         // *(x;1) -> x
00466         return recombine_pair_to_ex(*(seq.begin()));

and that could be somehow changed. And the other issue is ​here

00319         if (z.is_equal(*_num0_p))  // sin(0)       -> 0
00320             return _ex0;

so again if there were a way to have Pynac return "the right" zero... I think this is kind of philosophy difference is at the root of several little coercion buglets.

[rws]

Replying to kcrisman:

Here is another idea. In ​the Ginac source, we have

00464     } else if (seq_size==1 && overall_coeff.is_equal(_ex1)) {
00465         // *(x;1) -> x
00466         return recombine_pair_to_ex(*(seq.begin()));

This is currently in Pynac: ​https://github.com/pynac/pynac/blob/master/ginac/mul.cpp#L741

and that could be somehow changed. And the other issue is ​here

00319         if (z.is_equal(*_num0_p))  // sin(0)       -> 0
00320             return _ex0;

And this is currently ​https://github.com/pynac/pynac/blob/master/ginac/inifcns_trans.cpp#L430

The Pynac ticket for this on github is ​https://github.com/pynac/pynac/issues/4

[rws]

00464     } else if (seq_size==1 && overall_coeff.is_equal(_ex1)) {
00465         // *(x;1) -> x
00466         return recombine_pair_to_ex(*(seq.begin()));

​https://github.com/pynac/pynac/blob/master/ginac/mul.cpp#L713

There may be a general problem, not only for constants:

sage: 1.000000*(x+2)
x + 2
sage: 2.000000*(x+2)
2.00000000000000*x + 4.00000000000000

Is this intended?

[rws]

I think this can be generalized to -1,0,1:

sage: 0.0+5
5.00000000000000
sage: 0.0+x
x
sage: 0.0*x
0
sage: -1.0*x
-x
sage: -2.0*x
-2.00000000000000*x

sage: x^0.0
1
sage: x^1.0
x
sage: x^2.0
x^2.00000000000000
sage: x^-1.0
1/x
sage: x^-2.0
x^(-2.00000000000000)

[rws]

Conflicts with #18088, therefore a dependency.

---

New commits:

​4df15dd	18088: doctest for fix of pynac issue 20
​a853c03	Merge branch 'u/rws/inconsistency_with_0_0' of trac.sagemath.org:sage into pynac036
​e0e5e9e	12257: doctests

[rws]

New commits:

​31afae4

12257: only integer +/- 1 gets special treatment in Pynac

[kcrisman]

It should be that sin(1.0*pi) returns 0.0. We can fix this somehow. I'm not worried about printing.

Even without this patch, we now get (thanks to Pynac upgrade

sage: sin(1.0*pi)
sin(1.00000000000000*pi)

which is perhaps also not ideal, if we wanted 0.00000000000 as in the original intent (I guess) of this ticket. So while I agree that one bug is fixed, perhaps this could be repurposed for that - unless we want to say that sin(1.0*pi) is not exact so we don't actually know it's 0.0, but I feel we don't do that with other things. For instance, even after the Pynac upgrade in #18362, we have

sage: sin(0.0*pi)
0

which is really what is at issue here, I guess.

[rws]

Replying to kcrisman:

sage: sin(1.0*pi)
sin(1.00000000000000*pi)

which is perhaps also not ideal, if we wanted 0.00000000000 as in the original intent

I don't think you can expect 0.00000000000 but rather 1.22464679914735e-16 which is what you get with sin(1.0*pi).n(). I think it's Sage convention (?) that any FP value in an expression should have the effect of automatically applying N() to it.

So while I agree that one bug is fixed, perhaps this could be repurposed for that - unless we want to say that sin(1.0*pi) is not exact so we don't actually know it's 0.0

... even after the Pynac upgrade in #18362, we have

sage: 0.0*pi
0

which is really what is at issue here, I guess. Or?

Yes.

The original ticket was about 1.0. I added to this cases with -1.0 and 0.0 but when fixing stopped short of fixing the 0.0 cases because they seemed quite different from +/-1 and I first wanted to fix where I had a good overview without risk of introducing bugs. So, I think it better if we open a ticket for 0.0 because this case may need a separate discussion.

[rws]

Replying to rws:

Replying to kcrisman:

sage: sin(1.0*pi)
sin(1.00000000000000*pi)

which is perhaps also not ideal, if we wanted 0.00000000000 as in the original intent

I don't think you can expect 0.00000000000 but rather 1.22464679914735e-16 which is what you get with sin(1.0*pi).n(). I think it's Sage convention (?) that any FP value in an expression should have the effect of automatically applying N() to it.

No, I thought wrong, because I cannot find any function that expands an argument containing a FP number and a constant automatically (contrary to FP only). And I really agree it's wrong because the constant's precision is immediately limited as soon as the FP comes into it. But this is clearly another ticket and the introduced behaviour of sin(1.0*pi) with this ticket is exactly in line with all other functions and arguments at the moment.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​f137553	Merge branch 'develop' into t/12257/12257
​447ab20	12257: fix doctest

[rws]

The sin(1.0*pi) issue is now #18697.

[jdemeyer]

I think you need to be slightly more clear what you mean

only integer +/- 1 gets special treatment

I think something like the following would be more clear:

floating point numbers +/- 1.0 are treated differently from integers +/- 1

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​906736b

12257: cosmetics