Little fixes for a major speedup of join/meet matrices for FiniteLatticePoset

[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: attachment: trac_12476-lattice_join_matrix_speedup-fh.2.patch

Depends on #10998

Component: combinatorics

Keywords: poset, matrix, Cernay2012

Author: Florent Hivert, Nathann Cohen

Reviewer: Florent Hivert, Nicolas M. Thiéry

Merged: sage-5.0.beta6

Issue created by migration from https://trac.sagemath.org/ticket/12476

[nathanncohen]

comment:1

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

[nthiery]

Reviewer: Florent Hivert, Nicolas M. Thiéry

[nthiery]

comment:2

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!

[nthiery]

Dependencies: #10988

[hivert]

comment:4

There is a little problems with duplicated references in meet_matrix and join_matrix.

[hivert]

Attachment: trac_12476-lattice_join_matrix_speedup-fh.2.patch.gz

[hivert]

comment:5

Replying to @hivert:

There is a little problems with duplicated references in meet_matrix and join_matrix.

Fixed.

[nathanncohen]

comment:6

Nice catch ! There's no warning anymore when I generate the doc, so I guess the new patch can go too :-)

Nathann

[jdemeyer]

comment:7

How is this related to #10988 ???

[nthiery]

comment:8

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?

[jdemeyer]

Changed dependencies from #10988 to #10998

[jdemeyer]

comment:9

I guess you mean #10998 then...

[nthiery]

comment:10

Replying to @jdemeyer:

I guess you mean #10998 then...

Oh, yes, sorry!!!

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

[jdemeyer]

Merged: sage-5.0.beta6