Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#9615 closed defect (fixed)

doctest failures with lcalc functions in 4.5.2.alpha1

Reported by: ddrake Owned by: rishi
Priority: blocker Milestone: sage-4.5.2
Component: doctest coverage Keywords: lcalc
Cc: bober, craigcitro, cremona, mrubinst@…, mpatel, mvngu, rishi, ylchapuy, rbeezer Merged in: sage-4.5.2.rc0
Authors: Rishikesh, Leif Leonhardy Reviewers: Dan Drake, Mitesh Patel
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

Doctest failures in alpha1: https://groups.google.com/group/sage-release/msg/8807ed7073c6793f :

File "/scratch/scratch/schilly/sage/sage-4.5.2.alpha1/devel/sage/sage/
libs/lcalc/lcalc_Lfunction.pyx", line 780:
    sage: L.value(0.5)
Expected:
    0
Got:
    -1.28235854574334e-17
----------------------------------------------- 

This is related to #5396.

Attachments (4)

9615.patch (812 bytes) - added by rishi 11 years ago.
9615.2.patch (776 bytes) - added by mpatel 11 years ago.
Ticket number and somewhat more general comment in commit string. Apply only this patch.
trac_9615-lcalc_doctest_note.patch (794 bytes) - added by mpatel 11 years ago.
Add a note with the ticket number. Apply on top of 9615.2.patch
trac_9615-lcalc_doctest_note.2.patch (824 bytes) - added by mpatel 11 years ago.
Use Leif's better note. Replaces previous version. Apply on top of 9615.2.patch

Download all attachments as: .zip

Change History (20)

comment:1 Changed 11 years ago by leif

  • Cc rishi added

Just for the record: I get exactly the same number with

  • Ubuntu 7.10 x86, Pentium 4 (Northwood), gcc 4.2.1
  • Ubuntu 9.04 x86, Pentium 4 Prescott, gcc 4.3.3

From sage-release:

There is one numerical noise from #5396 ticket. I will write a patch which tests that L.value(0.5).abs() is less than 1e-14, instead of current value of 0.

The long messages giving out some data about L function are from a test function in lcalc library. It is helpful in testing. It just prints out to standard output. Please ignore them.

Rishi

comment:2 Changed 11 years ago by mpatel

  • Cc mpatel added

Changed 11 years ago by rishi

comment:3 Changed 11 years ago by rishi

  • Owner changed from mvngu to rishi

I did not check my email before creating a duplicate ticket and submitting a patch. Please close #9624

comment:4 Changed 11 years ago by mpatel

  • Cc bober craigcitro cremona mrubinst@… mvngu ylchapuy added
  • Status changed from new to needs_review

I'm changing the status to needs_review. Is that OK?

Also, if I don't do it, someone should prepend the ticket number in the patch commit string.

Changed 11 years ago by mpatel

Ticket number and somewhat more general comment in commit string. Apply only this patch.

comment:5 follow-ups: Changed 11 years ago by ddrake

  • Authors set to Rishikesh

The patch fixes the problem on a 32-bit Ubuntu Hardy VM which exhibits the doctest failure.

One question, though: is lcalc expected to return precisely zero, or a "noisy zero"? It seems unusual to me that software doing floating point calculations would return precisely zero on some platforms and a noisy zero on others.

If this kind of behavior is expected on 32-bit versus 64-bit machines, and if lcalc's answers are imprecise anyway, then I think this can get a positive review. (IOW, my ignorance about lcalc is what's keeping this from a positive review...)

comment:6 in reply to: ↑ 5 Changed 11 years ago by leif

Replying to ddrake:

[...] It seems unusual to me that software doing floating point calculations would return precisely zero on some platforms and a noisy zero on others.

If this kind of behavior is expected on 32-bit versus 64-bit machines, [...]

I can only imagine this is related to (unintentionally) different [default] rounding modes on different processors.

So this patch fixes the failing doctest (which is IMHO ok for the moment), but doesn't address its reason.

Unless the involved abs() (or <) is somehow broken, I can give this a positive review without applying the patch and actually testing it... ;-)

comment:7 follow-up: Changed 11 years ago by leif

The patched doctest lacks an explanation/reference back to this ticket, which I think would be appropriate in this case.

Not surprisingly passes all (long) tests in sage/libs/lcalc/ on Ubuntu 9.04 x86 (Pentium 4 Prescott, gcc 4.3.3), where the doctest previously failed.

I remember there was a single machine reported on sage-release to have a different result than my two 32-bit machines previously had, on which the patch should perhaps be tested, too (but afair also with abs(...) below 10-15).

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

  • Cc rbeezer added

Replying to leif:

I remember there was a single machine reported on sage-release to have a different result than my two 32-bit machines previously had, on which the patch should perhaps be tested, too (but afair also with abs(...) below 10-15).

[...] On 32-bit Ubuntu 10.04 on Pentium M I get the same results as Harald, though the non-zero value in the lcalc test is slightly different (~e-18). [...]

Rob

Rob, could you perhaps test this patch, too? (Though I don't expect it to fail on your machine either, since your result was even closer to zero.)

comment:9 in reply to: ↑ 5 Changed 11 years ago by rishi

In lcalc, the function being tested returns a complex number (double precision). It is probably the way different processors do the computations, that the doctest failures are occurring. The earlier doctest passed on all the machines I have access to.

The calculation of L-function is pretty involved, you can see the papers which are referred to in the patch in #5396.

Replying to ddrake:

The patch fixes the problem on a 32-bit Ubuntu Hardy VM which exhibits the doctest failure.

One question, though: is lcalc expected to return precisely zero, or a "noisy zero"? It seems unusual to me that software doing floating point calculations would return precisely zero on some platforms and a noisy zero on others.

If this kind of behavior is expected on 32-bit versus 64-bit machines, and if lcalc's answers are imprecise anyway, then I think this can get a positive review. (IOW, my ignorance about lcalc is what's keeping this from a positive review...)

comment:10 in reply to: ↑ 8 Changed 11 years ago by rbeezer

Same 32-bit system as before, applied ".2" patch. Then

sage -t -long "devel/sage-main/sage/libs/lcalc/lcalc_Lfunction.pyx"
	 [3.3 s]
 
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 3.3 seconds

Replying to leif:

Replying to leif:

I remember there was a single machine reported on sage-release to have a different result than my two 32-bit machines previously had, on which the patch should perhaps be tested, too (but afair also with abs(...) below 10-15).

[...] On 32-bit Ubuntu 10.04 on Pentium M I get the same results as Harald, though the non-zero value in the lcalc test is slightly different (~e-18). [...]

Rob

Rob, could you perhaps test this patch, too? (Though I don't expect it to fail on your machine either, since your result was even closer to zero.)

comment:11 in reply to: ↑ 5 ; follow-up: Changed 11 years ago by jhpalmieri

Replying to ddrake:

The patch fixes the problem on a 32-bit Ubuntu Hardy VM which exhibits the doctest failure.

For what it's worth, it also fixes it on two different skynet machines which had the failure: cicero (x86-Linux-pentium4-fc) and iras (ia64-Linux-suse).

Changed 11 years ago by mpatel

Add a note with the ticket number. Apply on top of 9615.2.patch

comment:12 follow-up: Changed 11 years ago by mpatel

  • Merged in set to sage-4.5.2.rc0
  • Reviewers set to Dan Drake
  • Status changed from needs_review to positive_review

Unless anyone objects, I'm changing the status to positive_review and merging

in 4.5.2.rc0. The latter, which someone should review, follows up on Leif's suggestion. Or should I exclude it?

comment:13 Changed 11 years ago by mpatel

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

comment:14 in reply to: ↑ 11 Changed 11 years ago by leif

Replying to jhpalmieri:

Replying to ddrake:

The patch fixes the problem on a 32-bit Ubuntu Hardy VM which exhibits the doctest failure.

For what it's worth, it also fixes it on two different skynet machines which had the failure: cicero (x86-Linux-pentium4-fc) and iras (ia64-Linux-suse).

Does iras default to 32-bit builds? (I only know it's running a 64-bit SuSE.)

I ask this because Harald also saw this doctest failure on a (newer) 64-bit processor running a 32-bit OS (Core2 quad, Ubuntu 8.10 x86).


I think someone should track down if this difference is an lcalc "feature" or perhaps a Sage library interface issue (and in the latter case open a new ticket).

comment:15 in reply to: ↑ 12 ; follow-up: Changed 11 years ago by leif

Replying to mpatel:

[...] which someone should review, follows up on Leif's suggestion. Or should I exclude it?

Well, I would have written

          ... # "noisy" zero on some platforms (see #9615)

But now it's too late...

Changed 11 years ago by mpatel

Use Leif's better note. Replaces previous version. Apply on top of 9615.2.patch

comment:16 in reply to: ↑ 15 Changed 11 years ago by mpatel

  • Authors changed from Rishikesh to Rishikesh, Leif Leonhardy
  • Reviewers changed from Dan Drake to Dan Drake, Mitesh Patel

Replying to leif:

But now it's too late...

I've attached an update with Leif's better comment. I'll merge

into 4.5.2.rc0. I'm adding Leif as an author and me as a reviewer.

Note: See TracTickets for help on using tickets.