Ticket #4373 (closed defect: fixed)

Opened 23 months ago

Last modified 15 months ago

[with patch, positive review] doctest failure in sage/algebras/group_algebra.py on 32 bit platforms

Reported by: mabshoff Owned by: davidloeffler
Priority: blocker Milestone: sage-3.2
Component: doctest Keywords:
Cc: mhansen Author(s): David Loeffler
Report Upstream: Reviewer(s): Michael Abshoff
Merged in: 3.2.alpha2 Work issues:

Description

sage -t  devel/sage/sage/algebras/group_algebra.py          
********************************************************************** 
File "/home/jaap/downloads/sage-3.2.alpha0/tmp/group_algebra.py", line 230: 
     sage: OG(FormalSum([ (1, G(2)), (2, RR(0.77)) ]), check=False) 
Expected: 
     [2 0] 
     [0 2] + 2*0.770000000000000 
Got: 
     2*0.770000000000000 + [2 0] 
     [0 2] 
**********************************************************************

Attachments

4373.patch Download (0.9 KB) - added by davidloeffler 22 months ago.
4373-new.patch Download (1.6 KB) - added by davidloeffler 22 months ago.

Change History

Changed 22 months ago by mabshoff

  • cc davidloeffler added

David,

since this is your patch any clue what is going on here?

Cheers,

Michael

Changed 22 months ago by mabshoff

  • priority changed from major to blocker

Changed 22 months ago by davidloeffler

Changed 22 months ago by davidloeffler

  • owner changed from mabshoff to davidloeffler
  • status changed from new to assigned

The failed doctest was just verifying that the "check" flag could be disabled, by using this to create a bogus group algebra element that is a formal sum of things that aren't in the right group. The string representation of such an element depends on the sort order of the elements, which is completely arbitrary and subject to changing between platforms, hence the failure here.

All that's really needed is to check that the method runs; it's inconceivable that it could run but not give the expected answer. So I've uploaded a 1-line patch that marks the doctest with "# random".

(I didn't use the "#64-bit" and "#32-bit" flags as then it would just break again next time someone changed the sort order of real numbers versus integer matrices, which is bound to happen before long.)

Changed 22 months ago by mabshoff

  • cc mhansen added; davidloeffler removed
  • summary changed from doctest failure in sage/algebras/group_algebra.py on 32 bit platforms to [with patch, needs review] doctest failure in sage/algebras/group_algebra.py on 32 bit platforms

Ok, I think in that situation then #random is warranted. But could you please replace the patch by a version that adds the explanation you just gave on the ticket why this fails so that in the future people looking at that doctest understand why. It would also be nice to mention this ticket number in the string, i.e. "the issue was also discussed at #4373".

Mike: FYI since you also had some comments about this code.

Cheers,

Michael

Changed 22 months ago by davidloeffler

Changed 22 months ago by davidloeffler

ok, here it is again, with a more informative comment on the doctest.

Changed 22 months ago by mabshoff

  • summary changed from [with patch, needs review] doctest failure in sage/algebras/group_algebra.py on 32 bit platforms to [with patch, positive review] doctest failure in sage/algebras/group_algebra.py on 32 bit platforms

Two small issues: The explanation should be before the doctest and indented and the '#' in the docstring needs to be escaped. I will take care of both small issues.

Cheers,

Michael

Changed 22 months ago by mabshoff

  • status changed from assigned to closed
  • resolution set to fixed

Merged in Sage 3.2.alpha2

Changed 15 months ago by davidloeffler

  • reviewer set to Michael Abshoff
  • merged set to 3.2.alpha2
  • author set to David Loeffler
Note: See TracTickets for help on using tickets.