Opened 13 years ago

Closed 13 years ago

#2279 closed defect (fixed)

[with additional patch, positive review] numerical noise? doctest failure in sage.rings.number_field.totallyreal.__selberg_zograf_bound with 2.10.2

Reported by: cremona Owned by: was
Priority: major Milestone: sage-2.10.3
Component: number theory Keywords:
Cc: Merged in:
Authors: Reviewers:
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

A fresh 64-bit install of 2.10.2 gives this (and only this) error with "make check":

sage -t  devel/sage-main/sage/rings/number_field/totallyreal.py**********************************************************************
File "totallyreal.py", line 410:
   sage: sage.rings.number_field.totallyreal.__selberg_zograf_bound(8,7)
Expected:
   15.851871776151311
Got:
   15.851871776151313
**********************************************************************
1 items had failures:
  1 of   1 in __main__.example_5
***Test Failed*** 1 failures.
For whitespace errors, see the file .doctest_totallyreal.py
        [1.7 s]
exit code: 256

----------------------------------------------------------------------
The following tests failed:


       sage -t  devel/sage-main/sage/rings/number_field/totallyreal.py

OS info:

jec@host-57-71%uname -a
Linux host-57-71 2.6.18.8-0.3-default #1 SMP Tue Apr 17 08:42:35 UTC 2007 x86_64 x86_64 x86_64 GNU/Linux
gcc version 4.1.2 20061115 (prerelease) (SUSE Linux)

Attachments (3)

8682.patch (1.7 KB) - added by jvoight 13 years ago.
trac-2279.patch (1.7 KB) - added by craigcitro 13 years ago.
trac-2279.pt2.patch (971 bytes) - added by craigcitro 13 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 13 years ago by mabshoff

  • Milestone set to sage-2.10.3

comment:2 Changed 13 years ago by jvoight

  • Summary changed from numerical noise? doctest failure in sage.rings.number_field.totallyreal.__selberg_zograf_bound with 2.10.2 to [with patch, needs review] numerical noise? doctest failure in sage.rings.number_field.totallyreal.__selberg_zograf_bound with 2.10.2

Deleted the function selberg_zograf_bound. Patch attached. JV

Changed 13 years ago by jvoight

comment:3 Changed 13 years ago by was

  • Summary changed from [with patch, needs review] numerical noise? doctest failure in sage.rings.number_field.totallyreal.__selberg_zograf_bound with 2.10.2 to [with patch, positive review] numerical noise? doctest failure in sage.rings.number_field.totallyreal.__selberg_zograf_bound with 2.10.2

Changed 13 years ago by craigcitro

comment:4 Changed 13 years ago by craigcitro

Same patch as the above, but it's a patch against 2.10.2, as opposed to the current working version of the code John and I are using (which is what the other patch above is against. ;) )

comment:5 Changed 13 years ago by mabshoff

  • Resolution set to fixed
  • Status changed from new to closed

Merged trac-2279.patch in Sage 2.10.3.rc0

comment:6 Changed 13 years ago by craigcitro

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:7 Changed 13 years ago by craigcitro

  • Summary changed from [with patch, positive review] numerical noise? doctest failure in sage.rings.number_field.totallyreal.__selberg_zograf_bound with 2.10.2 to [with additional patch, needs review] numerical noise? doctest failure in sage.rings.number_field.totallyreal.__selberg_zograf_bound with 2.10.2

In the fix posted above, I did two things:

  • removed selberg_zograf_bound, because it was causing numerical issues here and there, and wasn't actually used anywhere.
  • changed the default choice of totally real field of discriminant 5 (which gets added to the list by hand), because John had done that in his patch. Unfortunately, I switched a - for a + when doing it, and added a non-totally-real field.

The new patch I've attached fixes the second of these.

comment:8 Changed 13 years ago by cremona

I don't see the new patch. In fact the patches listed seem to be the same!

comment:9 Changed 13 years ago by craigcitro

Yep, I'm still waiting for my copy of rc0 to finish its sage -clone on sage.math. I was guessing it would finish before anyone looked at this ticket. ;)

Changed 13 years ago by craigcitro

comment:10 Changed 13 years ago by cremona

  • Summary changed from [with additional patch, needs review] numerical noise? doctest failure in sage.rings.number_field.totallyreal.__selberg_zograf_bound with 2.10.2 to [with additional patch, positive review] numerical noise? doctest failure in sage.rings.number_field.totallyreal.__selberg_zograf_bound with 2.10.2

Combined patches are fine. Note that 8682.patch is the same as trac-2279.patch, so may be ignored, but that trac-2279.pt2.patch needs to be applied after trac-2279.patch.

comment:11 Changed 13 years ago by mabshoff

  • Resolution set to fixed
  • Status changed from reopened to closed

Merged trac-2279.pt2.patch in Sage 2.10.3.rc1

Note: See TracTickets for help on using tickets.