Opened 12 years ago

Closed 12 years ago

#10041 closed defect (fixed)

Doctest failure in sage/tensor/differential_forms.py

Reported by: Mitesh Patel Owned by: Minh Van Nguyen
Priority: blocker Milestone: sage-4.6
Component: doctest coverage Keywords:
Cc: Jason Grout, Joris Vankerschaver, mhampton, Niles Johnson Merged in: sage-4.6.rc0
Authors: Joris Vankerschaver Reviewers: Niles Johnson
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Niles Johnson)

With the forthcoming 4.6.alpha2, I get this doctest failure on the Skynet machines cicero (x86-Linux-pentium4-fc) and fulvia (x86_64-SunOS-core2):

sage -t -long "devel/sage/sage/tensor/differential_forms.py"
**********************************************************************
File "/home/mpatel/build/cicero/sage-4.6.alpha2/devel/sage/sage/tensor/differential_forms.py", line 132:
    sage: cmp(F, G)
Expected:
    1
Got:
    -1

Related: #9650.

Apply

Attachments (1)

trac_10041_doctest_failure_differential_forms.patch (1.5 KB) - added by Joris Vankerschaver 12 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 Changed 12 years ago by Mitesh Patel

Description: modified (diff)

comment:2 Changed 12 years ago by Joris Vankerschaver

I will need to think some more about this. After hearing about this bug, I built 4.6.alpha2 on my Mac OS 10.6 machine and ran the doctests there, all of which passed without errors.

./sage -t -long "devel/sage/sage/tensor/differential_forms.py"
sage -t -long "devel/sage/sage/tensor/differential_forms.py"
	 [4.8 s]
 
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 4.8 seconds

I am currently building Sage on my Linux machine and hope to be able to reproduce that bug shortly. In the meantime, could this be due to cmp behaving differently on different platforms? This would seem very weird to me...

comment:3 Changed 12 years ago by Jason Grout

The problem seems to be in the coordinate patch code, I think.

Notice that:

sage: var('x,y,z')
(x, y, z)
sage: (x,y,z)<(y,x,z)
x < y

I think that comparing tuples of symbolic variables is not well-defined. Instead, you might compare strings (i.e., call str() on each tuple)

comment:4 Changed 12 years ago by Jason Grout

Do you really want to force an order on these things, or would it be enough to only define an equality-checking function?

comment:5 Changed 12 years ago by Joris Vankerschaver

Thanks Jason! I am uploading a patch to convert the tuples of coordinates to strings before comparing and I hope this solves the issue. I'm not sure we should ignore the order in coordinate tuples. For instance, T(T*Q) has natural bundle coordinates (q, p, q', p') whereas T*(TQ) naturally has coordinates (q, q', p, p'). The two bundles are isomorphic by switching the relevant coordinates, but it would be good to keep the distinction.

Time permitting, I will build Sage again on my Linux machine to make sure that the issue is resolved.

Changed 12 years ago by Joris Vankerschaver

comment:6 Changed 12 years ago by John Palmieri

The failing doctest now passes for me on cicero and fulvia. If someone else can confirm that it passes on some other machines (on ones where it passed before, we should make sure it still passes), feel free to give it a positive review.

Should this ticket be marked as "needs review"?

comment:7 in reply to:  5 ; Changed 12 years ago by Jason Grout

Status: newneeds_review

Replying to jvkersch:

Thanks Jason! I am uploading a patch to convert the tuples of coordinates to strings before comparing and I hope this solves the issue. I'm not sure we should ignore the order in coordinate tuples. For instance, T(T*Q) has natural bundle coordinates (q, p, q', p') whereas T*(TQ) naturally has coordinates (q, q', p, p'). The two bundles are isomorphic by switching the relevant coordinates, but it would be good to keep the distinction.

I meant, do you want to force a total order on the set of coordinates? In other words, what does (q,q',p,p')<(q,p,q',p') mean? Or do you just want to have equality testing, i.e., you can tell that (q,q',p,p') != (q,p,q',p')?

Regardless, I think resolving that design issue should be a separate ticket.

comment:8 in reply to:  7 Changed 12 years ago by Joris Vankerschaver

Replying to jason:

I meant, do you want to force a total order on the set of coordinates?

Good point -- that wouldn't make sense indeed.

Regardless, I think resolving that design issue should be a separate ticket.

I've put this up for review as #10053. This patch just takes out the calls to the __cmp__ member functions and replaces them with equivalent calls to __eq__.

comment:9 Changed 12 years ago by David Kirkby

I also see this on OpenSolaris on x86 on the clean 4.6.alpha1 code. I don't feel able to comment on the patches.

sage -t  -long devel/sage/sage/tensor/differential_forms.py
**********************************************************************
File "/export/home/drkirkby/sage-4.6.alpha2/devel/sage-main/sage/tensor/differential_forms.py", line 132:
    sage: cmp(F, G)
Expected:
    1
Got:
    -1
**********************************************************************
1 items had failures:
   1 of  11 in __main__.example_3
***Test Failed*** 1 failures.
For whitespace errors, see the file /export/home/drkirkby/.sage//tmp/.doctest_differential_forms.py

comment:10 Changed 12 years ago by John Cremona

The patch works for me (tests in that one file pass, which used to fail); I have not tested the whole library, just sage/tensor/*. This is on 32-bit ubuntu.

comment:11 Changed 12 years ago by Joris Vankerschaver

For what it's worth, make ptestlong tells me that all tests passed on my 64bit Linux machine (CentOS Linux 5.5).

comment:12 in reply to:  11 ; Changed 12 years ago by Niles Johnson

Reviewers: Niles Johnson

I applied this patch, and the one at #10053. With these, sage passes all (-long) doctests (linux, with Red Hat Enterprise). Note that the remaining issue, comment 9 above, appears because this patch addresses comparison only for coordinate patches, not differential form algebras or differential form elements. Since these are addressed by #10053, this ticket can get positive review when that one does.

comment:13 Changed 12 years ago by Niles Johnson

Description: modified (diff)

comment:14 in reply to:  12 Changed 12 years ago by Niles Johnson

Status: needs_reviewpositive_review

Replying to niles:

I applied this patch, and the one at #10053. With these, sage passes all (-long) doctests (linux, with Red Hat Enterprise). Note that the remaining issue, comment 9 above, appears because this patch addresses comparison only for coordinate patches, not differential form algebras or differential form elements. Since these are addressed by #10053, this ticket can get positive review when that one does.

#10053 now has positive review.

comment:15 Changed 12 years ago by Leif Leonhardy

Just for the record:

I now get this doctest error after upgrade-testing (#9896) with Sage 4.6.alpha3 (in-place upgrade from 4.5.3) on Ubuntu 9.04 x86, Pentium4 Prescott with SSE enabled (in CFLAGS and CXXFLAGS).

In the build from scratch on the same machine, with the same settings, I didn't get that error.

comment:16 Changed 12 years ago by Mitesh Patel

Authors: Joris Vankerschaver

comment:17 Changed 12 years ago by Mitesh Patel

Merged in: sage-4.6.rc0
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.