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 hivert)

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

Apply: trac_12476-lattice_join_matrix_speedup-fh.2.patch​

Attachments (1)

trac_12476-lattice_join_matrix_speedup-fh.2.patch​ (5.0 KB) - added by hivert 5 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 5 years ago by ncohen

  • Status changed from new to needs_review

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

comment:2 Changed 5 years ago by nthiery

  • 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 nthiery

  • Dependencies set to #10988
  • Description modified (diff)

comment:4 follow-up: Changed 5 years ago by hivert

  • 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 hivert

comment:5 in reply to: ↑ 4 Changed 5 years ago by hivert

  • 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 and join_matrix.

Fixed.

comment:6 Changed 5 years ago by ncohen

  • 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: Changed 5 years ago by jdemeyer

How is this related to #10988 ???

comment:8 in reply to: ↑ 7 Changed 5 years ago by nthiery

Replying to jdemeyer:

How is this related to #10988 ???

#10988 refactors a lot all the poset code; so we did not even try if this one would, by plain piece of luck, commute with it. Since #10988 is now in Sage (thanks btw!), does this have any importance?

comment:9 follow-up: Changed 5 years ago by jdemeyer

  • Dependencies changed from #10988 to #10998

I guess you mean #10998 then...

comment:10 in reply to: ↑ 9 Changed 5 years ago by nthiery

Replying to jdemeyer:

I guess you mean #10998 then...

Oh, yes, sorry!!!

A dependence upon #10988 would indeed be pretty scary :-)

comment:11 Changed 5 years ago by jdemeyer

  • 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.