#15664: Bug in IncidenceStructure.is_block_design
----------------------------------+----------------------------
Reporter: ncohen | Owner:
Type: defect | Status: needs_review
Priority: major | Milestone: sage-6.1
Component: combinatorics | Resolution:
Keywords: | Merged in:
Authors: Nathann Cohen | Reviewers:
Report Upstream: N/A | Work issues:
Branch: u/ncohen/15664 | Commit:
Dependencies: #15285 | Stopgaps:
----------------------------------+----------------------------
Changes (by ncohen):
* status: needs_work => needs_review
Old description:
> This bug has been noticed by Frederic in #15285 :
>
> {{{
> sage:
> IncidenceStructure([0,1,2],[[0,1],[0,2],[1,2]]*15).is_block_design()
> (True, [1, 3, 2, 15])
> sage: IncidenceStructure([0,1,2],[[0,1],[0,2],[1,2]]*15).parameters()
> (2, 3, 2, 15)
> }}}
>
> Turns out that the code of `is_block_design` was rather unclear, and
> rather... Costly. Here is a simpler, faster and more correct version in
> which the bug in solved.
>
> Before:
> {{{
> sage: D =
> IncidenceStructure(range(44),designs.steiner_quadruple_system(44))
> sage: time D.is_block_design()
> CPU times: user 99.26 s, sys: 0.06 s, total: 99.32 s
> Wall time: 99.40 s
> (True, [3, 44, 4, 1])
> }}}
>
> After:
> {{{
> sage: D =
> IncidenceStructure(range(44),designs.steiner_quadruple_system(44))
> sage: time D.is_block_design()
> CPU times: user 0.06 s, sys: 0.00 s, total: 0.06 s
> Wall time: 0.06 s
> (True, [3, 44, 4, 1])
> }}}
>
> Secondly, `IncidenceStructure.parameter` has a very tricky behaviour :
> {{{
> sage: D =
> IncidenceStructure(range(44),designs.steiner_quadruple_system(44))
> sage: D.is_block_design()
> (True, [3, 44, 4, 1])
> sage: D.parameters()
> (2, 44, 4, 21)
> }}}
>
> This is because `parameters()` take `t` as a parameter, and sets it to
> `2` by default. That's.... surprising `:-P`
>
> This patch adds a deprecation warning when this parameter is not set, and
> we will make it mandatory... in a while.
>
> Here it is !
>
> Nathann
New description:
This bug has been noticed by Frederic in #15285 :
{{{
sage: IncidenceStructure([0,1,2],[[0,1],[0,2],[1,2]]*15).is_block_design()
(True, [1, 3, 2, 15])
sage: IncidenceStructure([0,1,2],[[0,1],[0,2],[1,2]]*15).parameters()
(2, 3, 2, 15)
}}}
Turns out that the code of `is_block_design` was rather unclear, and
rather... Costly. Here is a simpler, faster and more correct version in
which the bug in solved.
Before:
{{{
sage: D =
IncidenceStructure(range(44),designs.steiner_quadruple_system(44))
sage: time D.is_block_design()
CPU times: user 99.26 s, sys: 0.06 s, total: 99.32 s
Wall time: 99.40 s
(True, [3, 44, 4, 1])
}}}
After:
{{{
sage: D =
IncidenceStructure(range(44),designs.steiner_quadruple_system(44))
sage: time D.is_block_design()
CPU times: user 0.06 s, sys: 0.00 s, total: 0.06 s
Wall time: 0.06 s
(True, [3, 44, 4, 1])
}}}
Secondly, `IncidenceStructure.parameter` has a very tricky behaviour :
{{{
sage: D =
IncidenceStructure(range(44),designs.steiner_quadruple_system(44))
sage: D.is_block_design()
(True, [3, 44, 4, 1])
sage: D.parameters()
(2, 44, 4, 21)
}}}
This is because `parameters()` take `t` as a parameter, and sets it to `2`
by default. That's.... surprising `:-P`
This patch adds a deprecation warning when this parameter is not set, and
we will make it mandatory... in a while.
Here it is !
Nathann
(this ticket took roughly 3hours of solid work)
--
--
Ticket URL: <http://trac.sagemath.org/ticket/15664#comment:5>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/groups/opt_out.
