Opened 4 years ago
Closed 4 years ago
#19036 closed enhancement (fixed)
Use vectors instead of tuples in Polyomino
Reported by:  vdelecroix  Owned by:  

Priority:  major  Milestone:  sage7.2 
Component:  combinatorics  Keywords:  
Cc:  slabbe  Merged in:  
Authors:  Vincent Delecroix  Reviewers:  Sébastien Labbé 
Report Upstream:  N/A  Work issues:  
Branch:  fb8f324 (Commits)  Commit:  fb8f324fa9e3cf16d22e3b91b8ee85e1f4aa6af9 
Dependencies:  Stopgaps: 
Description
As noticed in #18987, polyominoes would be much faster if they would store (immutable) integer vectors instead of tuples.
Change History (6)
comment:1 Changed 4 years ago by
 Branch set to u/vdelecroix/19036
 Commit set to 10f03e0cf2e11cf61785778d55c8a605d417a437
 Dependencies #18987 deleted
 Status changed from new to needs_review
comment:2 Changed 4 years ago by
 Milestone changed from sage6.9 to sage7.1
comment:3 Changed 4 years ago by
 Status changed from needs_review to needs_work
The patchbot reports doctests failures in file quatumino:
********************************************************************** 2 items had failures: 4 of 21 in sage.games.quantumino 2 of 13 in sage.games.quantumino.QuantuminoSolver.solve [76 tests, 6 failures, 20.14 s]  sage t long src/sage/games/quantumino.py # 6 doctests failed 
comment:4 Changed 4 years ago by
 Branch changed from u/vdelecroix/19036 to public/19036
 Commit changed from 10f03e0cf2e11cf61785778d55c8a605d417a437 to fb8f324fa9e3cf16d22e3b91b8ee85e1f4aa6af9
 Reviewers set to Sébastien Labbé
 Status changed from needs_work to needs_review
I confirm this branch makes things faster:
BEFORE:
sage: from sage.games.quantumino import QuantuminoSolver sage: q = QuantuminoSolver(0) sage: %timeit L = q.tiling_solver().rows() 1 loop, best of 3: 7.26 s per loop
AFTER:
sage: from sage.games.quantumino import QuantuminoSolver sage: q = QuantuminoSolver(0) sage: %timeit L = q.tiling_solver().rows() 1 loop, best of 3: 1.78 s per loop
I fixed the long time doctests errors. To me this is a positive review. I let Vincent change the status to positive review if he agrees with my commit.
Note: I rebased the branch of Vincent on top of most recent development version.
New commits:
22e1d10  Trac 19036: use vectors instead of tuples in polyomino

fb8f324  trac #19036: fixing long time doctests

comment:5 Changed 4 years ago by
 Milestone changed from sage7.1 to sage7.2
 Status changed from needs_review to positive_review
comment:6 Changed 4 years ago by
 Branch changed from public/19036 to fb8f324fa9e3cf16d22e3b91b8ee85e1f4aa6af9
 Resolution set to fixed
 Status changed from positive_review to closed
Note: See
TracTickets for help on using
tickets.
New commits:
Trac 19036: use vectors instead of tuples in polyomino