Opened 8 years ago

Closed 7 years ago

#12815 closed defect (fixed)

bugs in doctesting script for examples with tolerance

Reported by: mstreng Owned by: mvngu
Priority: critical Milestone: sage-5.7
Component: doctest coverage Keywords: tolerance doctest
Cc: jhpalmieri, kcrisman Merged in: sage-5.7.beta0
Authors: Jeroen Demeyer Reviewers: Robert Bradshaw
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #12493, #13899 Stopgaps:

Description (last modified by jdemeyer)

  1. When testing the file
    """
    EXAMPLES:
    
    The name blah is undefined::
    
        sage: blah+blah
        blahblah
    
    Here's a doctest that is out of tolerance::
    
        sage: 100+10000000000 # abs tol 0.1
        10000000000
        sage: 1+1
        2
    """
    

I get

File "/Users/marcostreng/tmp/test.sage", line 6:
    sage: blah+blah
Out of tolerance 10000000000.0 vs 10000000100.0

So the error of "blah is undefined" is not printed, while the "out of tolerance" is printed for the wrong test.

  1. Also, there is a bug when a test with tolerance is the last test:
    """
    EXAMPLES:
    
    Doctests with tolerance cannot be followed by empty lines::
    
        sage: 100+10000000000 # rel tol 0.1
        10000000000
    """
    

yields

  File "/Users/marcostreng/.sage//tmp/test_71331.py", line 58
    ... ''', res, 0.1, 're
    ^
SyntaxError: invalid syntax
  1. The following is documented to work, but actually doesn't work:
    """
    sage: 1e16     # relative tol 1e-10
    10^16
    
    """
    

Testing this gives

sage -t  "/home/jdemeyer/doctest/tolerance_fail.py"
**********************************************************************
File "/home/jdemeyer/doctest/tolerance_fail.py", line 2:
    sage: 1e16     # relative tol 1e-10
Expected '
    10^16
    ' got '1.00000000000000e16'
**********************************************************************

Attachments (1)

12815_tolerance_doc.patch (2.4 KB) - added by jdemeyer 7 years ago.

Download all attachments as: .zip

Change History (21)

comment:1 Changed 8 years ago by jhpalmieri

  • Cc jhpalmieri added

comment:2 Changed 7 years ago by jdemeyer

  • Dependencies changed from 12493 to #12493
  • Description modified (diff)
  • Priority changed from major to critical

comment:3 Changed 7 years ago by jdemeyer

A patch at #13899 documents the third bug in the ticket description.

comment:4 Changed 7 years ago by kcrisman

  • Cc kcrisman added

comment:5 Changed 7 years ago by jdemeyer

Issues 1 and 2 are fixed by #12415. Issue 3 might be "wontfix" but then the documentation needs to be changed.

comment:6 follow-ups: Changed 7 years ago by jdemeyer

Testing issue 3 with #12415 gives

sage -t /home/jdemeyer/doctest/tolerance_fail.py
**********************************************************************
File "/home/jdemeyer/doctest/tolerance_fail.py", line 2, in tolerance_fail
Failed example:
    1e16     # relative tol 1e-10
Expected:
    10^16
Got:
    1.00000000000000e16
**********************************************************************

comment:7 in reply to: ↑ 6 Changed 7 years ago by mstreng

Replying to jdemeyer:

Testing issue 3 with #12415 gives

Yes, and that is what the last block of changes of trac_13899-doctest-rebase.patch is for.

comment:8 in reply to: ↑ 6 ; follow-up: Changed 7 years ago by robertwb

Replying to jdemeyer:

Testing issue 3 with #12415 gives

sage -t /home/jdemeyer/doctest/tolerance_fail.py
**********************************************************************
File "/home/jdemeyer/doctest/tolerance_fail.py", line 2, in tolerance_fail
Failed example:
    1e16     # relative tol 1e-10
Expected:
    10^16
Got:
    1.00000000000000e16
**********************************************************************

I consider this a feature, not a bug. If we're going to allow arithmetic expressions, where do we draw the line?

sage: 1e16    # rel tol 1e-10
100000000^2
sage: 1e16    # rel tol 1e-10
4e15 + 6e15
sage: 1e16    # rel tol 1e-10
solve(1e16 - x, x)[0].rhs()

There is also the advantage of being able to write

sage: "%g is less than %g" % (sqrt(2), pi)   # rel tol .01
'1.41 is less than 3.14'

which is possible because we do a match on floating point literals.

comment:9 in reply to: ↑ 8 Changed 7 years ago by mstreng

Replying to robertwb:

I consider this a feature, not a bug.

+1 (for the reasons Robert mentions)

Jeroen, could appropriately change the documentation in trac_13899-doctest-rebase.patch?

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

I propose the following: we add a patch to this ticket to fix the documentation concerning issue 3 and close the first two issues as fixed by #12415.

comment:11 in reply to: ↑ 10 Changed 7 years ago by mstreng

Replying to jdemeyer:

I propose the following: we add a patch to this ticket to fix the documentation concerning issue 3 and close the first two issues as fixed by #12415.

Sounds good to me.

comment:12 Changed 7 years ago by jdemeyer

  • Dependencies changed from #12493 to #12493, #13899

Changed 7 years ago by jdemeyer

comment:13 Changed 7 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • Status changed from new to needs_review

Patch to clarify what tolerance testing does. Needs review.

comment:14 Changed 7 years ago by jdemeyer

  • Description modified (diff)

comment:15 Changed 7 years ago by jdemeyer

Could anybody please review this ticket?

comment:16 Changed 7 years ago by kcrisman

Is this patch "backwards"?

comment:17 follow-up: Changed 7 years ago by robertwb

  • Status changed from needs_review to positive_review

No, the patch is not backwards. The description looks good to me.

comment:18 Changed 7 years ago by jdemeyer

  • Milestone changed from sage-5.6 to sage-5.7
  • Reviewers set to Robert Bradshaw

comment:19 in reply to: ↑ 17 Changed 7 years ago by kcrisman

No, the patch is not backwards. The description looks good to me.

Thanks, I understand now; I was getting confused with #13899.

comment:20 Changed 7 years ago by jdemeyer

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