Ticket #4373 (closed defect: fixed)

Opened 5 years ago

Last modified 4 years 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 coverage Keywords:
Cc: mhansen Work issues:
Report Upstream: Reviewers: Michael Abshoff
Authors: David Loeffler Merged in: 3.2.alpha2
Dependencies: Stopgaps:

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 (930 bytes) - added by davidloeffler 5 years ago.
4373-new.patch Download (1.6 KB) - added by davidloeffler 5 years ago.

Change History

comment:1 Changed 5 years ago by mabshoff

  • Cc davidloeffler added

David,

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

Cheers,

Michael

comment:2 Changed 5 years ago by mabshoff

  • Priority changed from major to blocker

Changed 5 years ago by davidloeffler

comment:3 Changed 5 years 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.)

comment:4 Changed 5 years 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 5 years ago by davidloeffler

comment:5 Changed 5 years ago by davidloeffler

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

comment:6 Changed 5 years 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

comment:7 Changed 5 years ago by mabshoff

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

Merged in Sage 3.2.alpha2

comment:8 Changed 4 years ago by davidloeffler

  • Reviewers set to Michael Abshoff
  • Merged in set to 3.2.alpha2
  • Authors set to David Loeffler
Note: See TracTickets for help on using tickets.