Opened 11 years ago

Closed 7 years ago

#8588 closed defect (fixed)

list(SL(2,2)) is inconsistent with SL(2,2).list()

Reported by: nthiery Owned by: AlexGhitza
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: algebra Keywords: Special linear group, TestSuite
Cc: sage-combinat Merged in:
Authors: Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

sage: G = SL(2,2)
sage: TestSuite(G).run()
Failure in _test_enumerated_set_iter_list:
Traceback (most recent call last):
...
AssertionError: [1 1]
[0 1] != [1 0]
[0 1]
------------------------------------------------------------
The following tests failed: _test_enumerated_set_iter_list

sage: list(G)[2]
[1 1]
[0 1]
sage: G.list()[2]
[1 0]
[0 1]

Attachments (3)

trac-8588 (1.4 KB) - added by itaibn 9 years ago.
trac-8588-2 (1.5 KB) - added by itaibn 9 years ago.
trac-8588-3 (1.0 KB) - added by itaibn 9 years ago.

Download all attachments as: .zip

Change History (9)

Changed 9 years ago by itaibn

comment:1 Changed 9 years ago by itaibn

Just attached a fix.

Changed 9 years ago by itaibn

comment:2 Changed 9 years ago by itaibn

Fixed a typo and improved the formatting.

comment:3 Changed 9 years ago by nbruin

Are you sure this is a wise fix? This way, even starting to iterate on the elements of a matrix group involves constructing all its elements. The iterator method on matrix groups previously was the one inherited from sage.combinat.backtrack.TransitiveIdeal.__iter__, which seems a little more conservative.

In principle, people can write iter(SL(next_prime(10^5),2)).next() to get an example of a determinant 1 matrix. This doesn't need to be expensive (with the current code it is, though. It seems the semigroup_generators method is horribly inefficient). With the present patch it's guaranteed to be very expensive.

Iterators and lists have different use cases, so why should G.list() and list(G) give the same result? I think structures should be allowed to choose different enumeration methods depending on the application. G.list() knows it returns a list of all elements, so it can concentrate on speed and not worry about storage. list(G), which is just [g for g in G] asks G to produce an iterator over its elements, which can choose an enumeration method that saves memory and/or ensures that it's fast even when only a couple of elements are consumed.

It seems to me the correct fix is to amend TestSuite to not enforce that iter(G.list()) and iter(G) produce the elements in the same order.

comment:4 Changed 9 years ago by itaibn

The reason I this fix is that I assumed that whoever made that test knew what they were doing and had a good reason. I guess I should have checked if such a reason existed. I actually checked and found out that the test was made in #5891, a large patch where such a mistake could've slipped through.

Changed 9 years ago by itaibn

comment:5 Changed 7 years ago by aapitzsch

  • Milestone set to sage-duplicate/invalid/wontfix
  • Status changed from new to needs_review

I got

sage: G = SL(2,2)
sage: TestSuite(G).run()
sage: list(G)[2]
[0 1]
[1 1]
sage: G.list()[2]
[0 1]
[1 1]

comment:6 Changed 7 years ago by vbraun

  • Resolution set to fixed
  • Reviewers set to Volker Braun
  • Status changed from needs_review to closed
Note: See TracTickets for help on using tickets.