Opened 8 years ago

Closed 7 years ago

#11672 closed enhancement (fixed)

Some more doctests from the book "Calcul mathématique avec Sage"

Reported by: mmezzarobba Owned by: mvngu
Priority: minor Milestone: sage-5.7
Component: doctest coverage Keywords:
Cc: malb, Bouillaguet Merged in: sage-5.7.beta2
Authors: Marc Mezzarobba Reviewers: Martin Albrecht, Charles Bouillaguet
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by Bouillaguet)

The attached patch adds to the Sage testsuite most examples appearing Chapters 6 and 8 (Polynomials, Polynomial Systems) of the French book Calcul mathématique avec Sage. See #9395 for some background.

All tests pass with sage 5.5.

Attachments (1)

trac_11672_doctests_from_french_book.patch (26.4 KB) - added by mmezzarobba 7 years ago.
New version (last draft of the book, Sage 5.5)

Download all attachments as: .zip

Change History (28)

comment:1 Changed 8 years ago by mmezzarobba

  • Status changed from new to needs_review

comment:2 Changed 8 years ago by zimmerma

  • Cc malb added

Martin, please could you review this? This will help our book to be more stable with future versions of Sage. Thank you in advance (as a coauthor of the book, I'm quite reluctant to review this).

Paul

comment:3 Changed 8 years ago by malb

  • Reviewers set to Martin Albrecht
  • Status changed from needs_review to needs_work

Sure, I can take a look:

  • Please rename the patch to have the ".patch" suffix.
  • There might be an issue with the UTF-8, but we should try and wait if anybody complaints.
  • Do you really want to include an autogenerated file without any manual post processing? ("This file was *autogenerated* from sagebook.tex with sagetex.sty")
  • Please add your full name to the author field here on Trac.
  • I get some numerical noise errors:
    File "/mnt/usb1/scratch/malb/sage-4.7.2/devel/sage/sage/tests/french_book/mpoly.py", line 373:
        sage: [CDF['x'](p(y=ys[0][0])).roots() for p in J.gens()]
    Expected:
        [[(-0.6 - 1.30624677741e-16*I, 1), (0.6 + 1.30624677741e-16*I, 1)],
        [(0.6 - 3.13499226579e-16*I, 1), (2.6 + 3.13499226579e-16*I, 1)]]
    Got:
        [[(-0.6 - 1.30628991909e-16*I, 1), (0.6 + 1.30628991909e-16*I, 1)], 
        [(0.6 - 3.13509580582e-16*I, 1), (2.6 + 3.13509580582e-16*I, 1)]]
    

comment:4 Changed 8 years ago by zimmerma

Martin, on which platform do you get numerical noise errors? With Sage 4.7 on a 64-bit Core 2, I get no difference.

Paul

comment:5 Changed 8 years ago by malb

This was on sage.math (sorry, should have mentioned).

comment:6 Changed 7 years ago by Bouillaguet

  • Cc Bouillaguet added

I will try to make this go through

comment:7 Changed 7 years ago by zimmerma

thanks Charles!

Paul

comment:8 Changed 7 years ago by mmezzarobba

  • Authors set to Marc Mezzarobba
  • Priority changed from major to minor
  • Status changed from needs_work to needs_review

comment:9 Changed 7 years ago by mmezzarobba

Charles: Thanks!

Here is a new version of the patch, updated to the last draft of the book (which contains what should hopefully be the final version of the chapters in question...) and to Sage 5.5.

comment:10 Changed 7 years ago by Bouillaguet

  • Description modified (diff)
  • Milestone changed from sage-5.6 to sage-5.7
  • Reviewers changed from Martin Albrecht to Martin Albrecht, Charles Bouillaguet
  • Status changed from needs_review to positive_review

Looks good to me !

comment:11 Changed 7 years ago by jdemeyer

  • Status changed from positive_review to needs_work

On sage.math (Linux x86_64):

sage -t  -force_lib devel/sage/sage/tests/french_book/mpoly.py
**********************************************************************
File "/release/merger/sage-5.7.beta0/devel/sage-main/sage/tests/french_book/mpoly.py", line 366:
    sage: [CDF['x'](p(y=ys[0][0])).roots() for p in J.gens()]
Expected:
    [[(-0.6 - 1.30624...e-16*I, 1), (0.6 + 1.30624...e-16*I, 1)],
    [(0.6 - 3.13499...e-16*I, 1), (2.6 + 3.13499...e-16*I, 1)]]
Got:
    [[(-0.6 - 1.30628991909e-16*I, 1), (0.6 + 1.30628991909e-16*I, 1)], [(0.6 - 3.13509580582e-16*I, 1), (2.6 + 3.13509580582e-16*I, 1)]]
**********************************************************************

comment:12 Changed 7 years ago by mmezzarobba

  • Status changed from needs_work to needs_review

I am unable to reproduce the failure you get on sage.math (though I am working on Linux x86_64 too), so I just reduced the number of digits taken into account in this test again.

comment:13 Changed 7 years ago by Bouillaguet

  • Status changed from needs_review to positive_review

I confirm that I ran the tests before giving the earlier positive review. It did work fine on my core i7-3615QM CPU running OS X 10.7.5...

The new patch also gets a positive review (the tests pass).

comment:14 Changed 7 years ago by jdemeyer

  • Status changed from positive_review to needs_work

On bsd (OS X 10.6 x86_64):

sage -t  --long -force_lib devel/sage/sage/tests/french_book/mpoly.py
**********************************************************************
File "/Users/buildbot/build/sage/bsd-1/bsd_full/build/sage-5.7.alpha1/devel/sage-main/sage/tests/french_book/mpoly.py", line 373:
    sage: J.variety(RDF)
Expected:
    [{y: 396340.89..., x: 26.61226...}]
Got:
    [{y: 396340.890167, x: -54.9445477048}]
**********************************************************************

On arando (Linux Ubuntu 12.04 i686):

sage -t  --long -force_lib devel/sage/sage/tests/french_book/mpoly.py
**********************************************************************
File "/var/lib/buildbot/build/sage/arando-1/arando_full/build/sage-5.7.alpha1/devel/sage-main/sage/tests/french_book/mpoly.py", line 373:
    sage: J.variety(RDF)
Expected:
    [{y: 396340.89..., x: 26.61226...}]
Got:
    [{y: 396340.890167, x: -46.7888621592}]
**********************************************************************

comment:15 Changed 7 years ago by zimmerma

I wonder why we get different results for x. If all computations are based on IEEE 754, we should get the same (wrong) value. Is it possible to know which components of Sage are used for J.variety(RDF) on each platform? On mine I get:

sage: get_systems('J.variety(RDF)')                          
['MPFI', 'Singular', 'numpy', 'ginac']

Since MPFI is based on MPFR, MPFI should be architecture independent, but I wonder if the same applies to Singular, numpy and ginac...

Paul

Changed 7 years ago by mmezzarobba

New version (last draft of the book, Sage 5.5)

comment:16 follow-up: Changed 7 years ago by mmezzarobba

Paul: Could it be that whatever is used is compiled using extended precision on some platforms? Another strange thing, though, is that

sage: J.variety(Reals(p))
[{y: -1.00000001350726, x: -1929.60019649825}, {y: -0.999999986492719, x: 1929.62017010095}, {y: 396340.890166545, x: 10.3009522157620}]

finds 3 solutions for p as low as 39...

Anyway, this test doesn't make much sense, I shouldn't have included this test in the first place. Here is a new version of the patch without that test. I took the opportunity to ignore the last few significant digits in another numerical result that might change a bit in the future.

comment:17 Changed 7 years ago by mmezzarobba

  • Status changed from needs_work to needs_review

comment:18 follow-up: Changed 7 years ago by zimmerma

Paul: Could it be that whatever is used is compiled using extended precision on some platforms?

yes, or with a different rounding precision. Anyway, it might be worth tracking down this issue. Jeroen, is there a way to trace the calls to the different Sage components (MPFI, Singular, numpy, ginac)?

Paul

comment:19 in reply to: ↑ 16 ; follow-up: Changed 7 years ago by Bouillaguet

Replying to mmezzarobba:

Paul: Could it be that whatever is used is compiled using extended precision on some platforms? Another strange thing, though, is that

sage: J.variety(Reals(p))
[{y: -1.00000001350726, x: -1929.60019649825}, {y: -0.999999986492719, x: 1929.62017010095}, {y: 396340.890166545, x: 10.3009522157620}]

finds 3 solutions for p as low as 39...

Anyway, this test doesn't make much sense, I shouldn't have included this test in the first place. Here is a new version of the patch without that test. I took the opportunity to ignore the last few significant digits in another numerical result that might change a bit in the future.

Well, the test obviously indicate that there is a problem somewhere, and I tend to believe that we should keep the test and track the problem.

Either your computation is ill-conditioned, and the test should be removed from Sage (and probably also from the book), or there is a component of sage that misbehaves. We should understand what the situation is, and act accordingly. These tests have been there for 1.5 years, so we are not in a hurry. Maybe we could compare intermediate results on different machines?

comment:20 in reply to: ↑ 18 Changed 7 years ago by jdemeyer

Replying to zimmerma:

Jeroen, is there a way to trace the calls to the different Sage components (MPFI, Singular, numpy, ginac)?

I don't think so. You could of course use gdb and set suitable breakpoints...

comment:21 in reply to: ↑ 19 Changed 7 years ago by mmezzarobba

Replying to Bouillaguet:

Either your computation is ill-conditioned, and the test should be removed from Sage (and probably also from the book), or there is a component of sage that misbehaves. We should understand what the situation is, and act accordingly.

Sorry for the lack of context. Yes, the computation is ill-conditioned, that's deliberate (see http://purple.lateralis.org/sagebook-r1014.pdf, p. 210-211), and all three results above are completely wrong (the correct result is x=6.305...).

As Paul noted, in an ideal world, this code should nontheless yield the same result everywhere, so it might have made sense to test that the result didn't change over time. But there are a number of reasonable explanations for what we observe, and (as far as I can tell) in Sage, floating-point results are usually interpreted as mere approximations that should agree with the correct answer "up to some numerical noise" rather than well-defined exact results. So I am in favor of dropping this testcase—unless the policy is that machine-precision floating-point computations should yield exactly the same result on all platforms.

(Of course, this doesn't mean we shouldn't try to understand exactly what is going on!)

comment:22 follow-up: Changed 7 years ago by jdemeyer

Or, add

J.variety(RDF)    # random

comment:23 Changed 7 years ago by zimmerma

I have created #13980 to keep track of the numerical noise issue, so that we can make progress on this ticket independently.

Paul

comment:24 in reply to: ↑ 22 Changed 7 years ago by mmezzarobba

Replying to jdemeyer:

Or, add J.variety(RDF) # random

Is that better? (Given that many examples from the book are already excluded from the tests for various reasons.) Please tell me if you would like me to update the test file.

comment:25 Changed 7 years ago by jdemeyer

It was only a suggestion, do as you wish.

comment:26 Changed 7 years ago by Bouillaguet

  • Status changed from needs_review to positive_review

comment:27 Changed 7 years ago by jdemeyer

  • Merged in set to sage-5.7.beta2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.