Opened 5 years ago
Closed 5 years ago
#12476 closed defect (fixed)
Little fixes for a major speedup of join/meet matrices for FiniteLatticePoset
Reported by: | hivert | Owned by: | sage-combinat |
---|---|---|---|
Priority: | major | Milestone: | sage-5.0 |
Component: | combinatorics | Keywords: | poset, matrix, Cernay2012 |
Cc: | Merged in: | sage-5.0.beta6 | |
Authors: | Florent Hivert, Nathann Cohen | Reviewers: | Florent Hivert, Nicolas M. Thiéry |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #10998 | Stopgaps: |
Description (last modified by )
Before
sage: %time posets.BooleanLattice(8) CPU times: user 10.42 s, sys: 0.01 s, total: 10.43 s Wall time: 10.51 s Finite lattice containing 256 elements
After
sage: %time posets.BooleanLattice(8) CPU times: user 0.75 s, sys: 0.01 s, total: 0.76 s Wall time: 0.84 s Finite lattice containing 256 elements
Attachments (1)
Change History (12)
comment:1 Changed 5 years ago by
- Status changed from new to needs_review
comment:2 Changed 5 years ago by
- Reviewers set to Florent Hivert, Nicolas M. Thiéry
- Status changed from needs_review to positive_review
- Summary changed from Join matrix is unreasonably slow to Little fixes for a major speedup of join/meet matrices for FiniteLatticePoset
I folded the reviewer's patch in the original patch. All test pass and the new code is semantically equivalent, but way faster. Positive review! Thanks!
comment:3 Changed 5 years ago by
- Dependencies set to #10988
- Description modified (diff)
comment:4 follow-up: ↓ 5 Changed 5 years ago by
- Status changed from positive_review to needs_work
There is a little problems with duplicated references in meet_matrix
and join_matrix
.
Changed 5 years ago by
comment:5 in reply to: ↑ 4 Changed 5 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
Replying to hivert:
There is a little problems with duplicated references in
meet_matrix
andjoin_matrix
.
Fixed.
comment:6 Changed 5 years ago by
- Status changed from needs_review to positive_review
Nice catch ! There's no warning anymore when I generate the doc, so I guess the new patch can go too :-)
Nathann
comment:7 follow-up: ↓ 8 Changed 5 years ago by
How is this related to #10988 ???
comment:8 in reply to: ↑ 7 Changed 5 years ago by
comment:9 follow-up: ↓ 10 Changed 5 years ago by
- Dependencies changed from #10988 to #10998
I guess you mean #10998 then...
comment:10 in reply to: ↑ 9 Changed 5 years ago by
comment:11 Changed 5 years ago by
- Merged in set to sage-5.0.beta6
- Resolution set to fixed
- Status changed from positive_review to closed
Note: See
TracTickets for help on using
tickets.
Helloooooooooooo !!!
I added two modifications (which did not appear to make much of a difference) but also make the code slightly easier to read. There already was a reference, I added another, no one is hurt
:-)
If you are ok with this second set of modifications you can set the ticket to "positive_review".
Nathann