"Wrong results" or "Bad input" ?

[Nathann Cohen]

Hello everybody,

I write this email because I would like your opinion on a ticket that is being
written.

- In #17637 [1], a stopgap (warning) was added in `IntegerListsLex`, to warn
  users against wrong results returned by this function.

- In #17898 [2], this stopgap is being removed, under the belief that those
  results are only caused by *bad input* (and thus should not be considered as a
  bug)

I believe that the description of #17892 [2] fairly describes the positions of
the authors, while the 3rd comment [3] on the same ticket describes mine.

It seems, however, that our main point of disagreement comes from whether those
those wrong results should be considered as wrong results. Indeed, the
documentation of `IntegerListsLex` reads:

       Caveat: with the current implementation, the constraints should
       satisfy the following conditions:

       - The upper and lower bounds themselves should satisfy the
         slope constraints.

       - The maximal and minimal slopes values should not be equal.

       - The maximal and minimal part values should not be equal.

       Those conditions are not checked by the algorithm, and the
       result may be completely incorrect if they are not satisfied:

I tried to write a code that checks the consistency of input, and had to give up
after two hours. There are too many cases to handle. Thus, I do not believe that
we cannot expect the user to do it him(her)self.

I also do not believe that such a note, buried in the doc, is a sufficient
warning.

I stand for the stopgap, which warns anybody who uses the function that it
is known to return wrong results.

Could you please share your opinion on whether we should keep or remove this
stopgap? We cannot persist in a situation where one ticket reverts what was done
by another.

Thanks,

Nathann

[1] http://trac.sagemath.org/ticket/17637
[2] http://trac.sagemath.org/ticket/17898
[3] http://trac.sagemath.org/ticket/17898#comment:3

[Jeroen Demeyer]

On 2015-03-05 09:58, Nathann Cohen wrote:
> Could you please share your opinion on whether we should keep or remove this
> stopgap?

Normally I would say: just write a few lines of code to check the input
conditions and be done with it. That would obviously be the best solution.

Since this seems to be hard, I tend to agree with the stopgap: it's
dangerous to use that function, so it's good that users are warned.

As suggested in the ticket, a partial solution would be to check for
certain "known good" cases, and don't display the stopgap in those cases.

[Dima Pasechnik]

I am for a stopgap. While there could potentially be cases when the
input is to hard to check for correctness, this is obviously not the
case here.

[Travis Scrimshaw]

   Then lets put a stopgap when Sage starts, since Sage has many known bugs but we don't have good ways to check against those inputs (which is worse because the vast majority of these aren't even documented). It's dangerous to use Sage.

Yes I know I'm being slightly ridiculous, but it does hold by your logic.

Travis

[Jeroen Demeyer]

On 2015-03-05 19:08, Travis Scrimshaw wrote:
> Sage has many known
> bugs but we don't have good ways to check against those inputs

Is that really true? Are there many known (i.e. reported on Trac)
examples where Sage returns *wrong* output?

[Volker Braun]

In the long run there is IMHO no alternative to writing proper input validation. Yes it may be hard, but it is mandatory. If you put something in the global namespace then users will find it and try it out. You can't expect everybody to read the documentation first. Typical exploration is to take an example and then modify some of the parameters. If that then quietly returns the wrong answer then it looks like Sage is broken. 

Also, are you 100% sure that your code that builds on top of IntegerListsLex is 100% correct and always passes valid input, even in edge cases? 

[Robert Bradshaw]

I think proper input validation (at least not silently accepting input
that will produce wrong answers) is a wortwhile bar to set for code in
the global namespace.

> --
> You received this message because you are subscribed to the Google Groups
> "sage-devel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to sage-devel+...@googlegroups.com.
> To post to this group, send email to sage-...@googlegroups.com.
> Visit this group at http://groups.google.com/group/sage-devel.
> For more options, visit https://groups.google.com/d/optout.

[Travis Scrimshaw]

   Yet this stopgap is not in a class in the global namespace, but a low-level class that almost always gets passed valid input. I am for having checks for valid input (and most of the time it does error out), but there is only one place I know of in the global namespace that can get invalid input ( Partitions(k, max_slope=m) with m >= 0 ). We have years of testing code which does pass valid input into IntegerListsLex that gives correct results (and it sometimes works on invalid input as well).

Here are some tickets where Sage silently returns invalid results (from 2 minutes of looking through the bugs):

- http://trac.sagemath.org/ticket/15249 (lazy power series)
- http://trac.sagemath.org/ticket/16190 (polynomials)
- http://trac.sagemath.org/ticket/8111 (gcd)
- http://trac.sagemath.org/ticket/12731 (integration)

So at the very least all of these should have top-level stopgap warning messages since there seems to be no easy way to tell when input. I am sure there are many others.

Travis

[Michael Orlitzky]

On 03/06/2015 04:51 PM, Travis Scrimshaw wrote:
> Yet this stopgap is not in a class in the global namespace, but a
> low-level class that almost always gets passed valid input. I am for having
> checks for valid input (and most of the time it does error out), but there
> is only one place I know of in the global namespace that can get invalid

> input ( Partitions(k, max_slope=m) with m >= 0 ). We have *years* of

> testing code which does pass valid input into IntegerListsLex that gives
> correct results (and it sometimes works on invalid input as well).
>
> Here are some tickets where Sage silently returns invalid results (from 2
> minutes of looking through the bugs):
>

This might be the first bug I ever reported:

sage: m = matrix([ [(-3/10), (1/5), (1/10)],
....: [(1/5), (-2/5), (2/5)],
....: [(1/10), (1/5), (-1/2)] ])
sage:
sage: n = matrix([ [-0.3, 0.2, 0.1],
....: [0.2, -0.4, 0.4],
....: [0.1, 0.2, -0.5] ])
sage:
sage: m.rank()
2
sage: n.rank()
3
sage: m == n
True

[Volker Braun]

It seems that we agree that input should be validated.

Should IntegerListLex be in the global namespace at all? The documentation says: "This is a generic low level tool. The interface has been designed with efficiency in mind. It is subject to incompatible changes in the future." 

On Friday, March 6, 2015 at 10:51:40 PM UTC+1, Travis Scrimshaw wrote:

- http://trac.sagemath.org/ticket/15249 (lazy power series)

It seems that a stopgap would be redundant there.

sage: L.<x> = LazyPowerSeriesRing(QQ)

sage: 2*x

Uninitialized lazy power series

[P Purkayastha]

This is a serious bug. Is there a ticket for this? 

[William Stein]

I don't consider this a bug. Rank is not a very meaningful for
matrices with floating point entries.

-- William


--
William (http://wstein.org)

[Michael Orlitzky]

On 03/06/2015 08:10 PM, P Purkayastha wrote:
>>
>>
> This is a serious bug. Is there a ticket for this?
>

It sort of falls under this one:

http://trac.sagemath.org/ticket/7392

even though a fix for that ticket might not affect some similar examples.

[William Stein]

If you change the definition of rank, make sure to not do the
following, like I just thought I should do for a few minutes:

sage: n = matrix([ [-0.3, 0.2, 0.1], [0.2, -0.4, 0.4], [0.1, 0.2, -0.5] ])
sage: a = n.numpy()
sage: import numpy
sage: numpy.rank(a)
2

It looks right... but it's 2 because it's a 2-dimensional array :-)

sage: numpy.rank(numpy.zeros((1,2,4)))
3

Anyway, now you see how numerical people use the word "rank"...

[P Purkayastha]

Well, something is clearly very wrong with either rank() or with ==. Since the matrices can be changed from one ring to another and they give True on equality it makes no sense for them to give different ranks.



I don't think a typical user wants to go deep into mathematics and try to figure out what should be the correct way to determine the rank of a matrix. It should give the correct answer. Otherwise the method should be removed from fields that can give incorrect results.

sage: m.change_ring(RDF)
[-0.3 0.2 0.1]
[ 0.2 -0.4 0.4]
[ 0.1 0.2 -0.5]
sage: n.change_ring(QQ)
[-3/10 1/5 1/10]
[ 1/5 -2/5 2/5]
[ 1/10 1/5 -1/2]
sage: m.change_ring(RDF).rank()
3
sage: n.change_ring(QQ).rank()
2

[William Stein]

"Equal" in Sage means "have equal images under the canonical parent
object that the both map to". For example, we have

sage: Mod(3,5) == 3
True

but the number 3 modulo 5 (which is an element of Z/5Z) has many
properties that are different from that of the integer 3.

>
>
>
> I don't think a typical user wants to go deep into mathematics and try to
> figure out what should be the correct way to determine the rank of a matrix.
> It should give the correct answer. Otherwise the method should be removed
> from fields that can give incorrect results.

I tend to agree -- Having rank be removed or raise an error and
suggest that if they want to compute ranks they must use an exact ring
(e.g., QQ) would make a lot of sense to me. There is no sensible
reason to try to compute the rank of a matrix with floating point
entries. It's slow and completely pointless. If they really are
trying to do numerical analysis, people should learn about QR
decompositions (etc.), and if they are not, they should use an exact
ring.

>
> sage: m.change_ring(RDF)
> [-0.3 0.2 0.1]
> [ 0.2 -0.4 0.4]
> [ 0.1 0.2 -0.5]
> sage: n.change_ring(QQ)
> [-3/10 1/5 1/10]
> [ 1/5 -2/5 2/5]
> [ 1/10 1/5 -1/2]
> sage: m.change_ring(RDF).rank()
> 3
> sage: n.change_ring(QQ).rank()
> 2
>

--
William (http://wstein.org)

[Volker Braun]

On Saturday, March 7, 2015 at 3:36:18 AM UTC+1, William wrote:

I tend to agree -- Having rank be removed or raise an error and
suggest that if they want to compute ranks they must use an exact ring

Its still educational, to teach about floating point computation and so on. We could raise a warning...

[Simon King]

I agree. I think a warning message menntioning appropriate methods over
inexact fields and advising to use exact rings would be ideal.

Best regards,
Simon

[John Cremona]

I think that many users would consider 1.2 to be an exact number. I
have certainly had to explain to students that if the answer to an
exact calculation is 6/5 then it is better not to convert to decimal,
despite what they were taught at school. Of course I understand
what floating point numbers are, so don't bother telling me! But at
school, and in mathematics (as opposed to computational mathematics),
1.2 and 6/5 are two different ways of writing the exact same number.

We cannot really win with this one. From what I wrote above it is
tempting to convert "simple" decimal literals into some exact ring on
input, but then someone will input 0.333 and be disappointed that Sage
did not guess that they meant 1/3.

John


>
> Best regards,
> Simon

[Ralf Stephan]

On Saturday, March 7, 2015 at 1:52:40 PM UTC+1, John Cremona wrote:

I think that many users would consider 1.2 to be an exact number.   I
have certainly had to explain to students that if the answer to an
exact calculation is 6/5 then it is better not to convert to decimal,
despite what they were taught at school.    Of course I understand
what floating point numbers are, so don't bother telling me!  But at
school, and in mathematics (as opposed to computational mathematics),
1.2 and 6/5 are two different ways of writing the exact same number.

+1 

We cannot really win with this one.  From what I wrote above it is
tempting to convert "simple" decimal literals into some exact ring on
input, but then someone will input 0.333 and be disappointed that Sage
did not guess that they meant 1/3.

 I don't think so. No one could seriously object against it being 333/1000.

See also http://trac.sagemath.org/ticket/8896 with similar doubts.

[William Stein]

On Sat, Mar 7, 2015 at 4:52 AM, John Cremona <john.c...@gmail.com> wrote:
> On 7 March 2015 at 08:50, Simon King <simon...@uni-jena.de> wrote:
>> On 2015-03-07, Volker Braun <vbrau...@gmail.com> wrote:
>>> On Saturday, March 7, 2015 at 3:36:18 AM UTC+1, William wrote:
>>>>
>>>> I tend to agree -- Having rank be removed or raise an error and
>>>> suggest that if they want to compute ranks they must use an exact ring
>>>
>>>
>>> Its still educational, to teach about floating point computation and so on.
>>> We could raise a warning...
>>
>> I agree. I think a warning message menntioning appropriate methods over
>> inexact fields and advising to use exact rings would be ideal.
>
> I think that many users would consider 1.2 to be an exact number. I
> have certainly had to explain to students that if the answer to an
> exact calculation is 6/5 then it is better not to convert to decimal,
> despite what they were taught at school. Of course I understand
> what floating point numbers are, so don't bother telling me! But at
> school, and in mathematics (as opposed to computational mathematics),
> 1.2 and 6/5 are two different ways of writing the exact same number.

In case you want to experience what Sage would feel like with 1.2
becoming exact, do this:

RealNumber0 = RealNumber; RealNumber = lambda x : QQ(RealNumber0(x))

Then

sage: .3
3/10

sage: n = matrix([ [-0.3, 0.2, 0.1],[0.2, -0.4, 0.4], [0.1, 0.2, -0.5] ])

sage: n.rank()
2
sage: n

[-3/10 1/5 1/10]
[ 1/5 -2/5 2/5]
[ 1/10 1/5 -1/2]

etc. So actually *implementing* what you're talking about above is
an easy one-liner.

>
> We cannot really win with this one. From what I wrote above it is
> tempting to convert "simple" decimal literals into some exact ring on
> input, but then someone will input 0.333 and be disappointed that Sage
> did not guess that they meant 1/3.

I suspect that most users of computing software outside of pure math
(e.g., in engineering, applied math, sciences, etc.) expect pretty
much everything to just get rounded to double precision floating like
it does on (1) most calculators, and -- more importantly -- like it
does in (2) Matlab.

Matlab is in fact genuinely much worse than Sage here, in my opinion.
It has a way to represent rational numbers as fractions, and lets you
work with matrices involving them. It's funny because under the hood
it does *everything* using floats, so some answers are completely
wrong (if the numbers were rationals, as they are displayed). Here's
an example; it's a simplified version of something that confused the
hell out of a grad student once a few years ago. Below I compute the
rref of the same matrix over QQ in Octave, Matlab, and Sage, and get
three completely different answers!


In Octave:

octave:1> format rat;
octave:2> a = [-86/17,40/29,-68/43,-20/11;-24/17,-1/38,-2/25,49/17]
a = -86/17 40/29 -68/43 -20/11 -24/17 -1/38 -2/25 49/17
octave:3> rref(a)
ans = 1 0 155/2122 -725/384 0 1 -152/173 -6553/795

In Matlab:
>> format rat;
>> a = [-86/17,40/29,-68/43,-20/11;-24/17,-1/38,-2/25,49/17]
a = -86/17 40/29 -68/43 -20/11 -24/17 -1/38 -2/25 49/17
>> rref(a)
ans = 1 0 13/178 -725/384 0 1 -152/173 -1426/173

sage: F = matrix(2,[-86/17, 40/29, -68/43, -20/11, -24/17, -1/38, -2/25, 49/17])
sage: F.rref()
[ 1 0 306034/4189705 -404710/214357]
[ 0 1 -18405604/20948525 -30037214/3644069]


Caveat: this example is from 2009 -- see
http://linear.ups.edu/sage/linear-tutorial-beezer-sd15.pdf

>
> John
>
>
>>
>> Best regards,
>> Simon
>>
>>
>> --
>> You received this message because you are subscribed to the Google Groups "sage-devel" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+...@googlegroups.com.
>> To post to this group, send email to sage-...@googlegroups.com.
>> Visit this group at http://groups.google.com/group/sage-devel.
>> For more options, visit https://groups.google.com/d/optout.
>
> --
> You received this message because you are subscribed to the Google Groups "sage-devel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+...@googlegroups.com.
> To post to this group, send email to sage-...@googlegroups.com.
> Visit this group at http://groups.google.com/group/sage-devel.
> For more options, visit https://groups.google.com/d/optout.

--
William (http://wstein.org)

[Ralf Stephan]

On Friday, March 6, 2015 at 10:51:40 PM UTC+1, Travis Scrimshaw wrote:

- http://trac.sagemath.org/ticket/12731 (integration)

A stopgap here would still allow destructive side effects. I have proposed

a solution.