#10530: De Bruijn Sequence construction for combinat
-----------------------------+----------------------------------------------
Reporter: eviatarbach | Owner: sage-combinat
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-4.6.1
Component: combinatorics | Keywords:
Author: Eviatar Bach | Upstream: N/A
Reviewer: | Merged:
Work_issues: |
-----------------------------+----------------------------------------------
Changes (by nthiery):
* status: needs_review => needs_work
Comment:
Hi!
I just had a quick look at your patch. This sounds good!
A couple micro comments (before an actual review):
- CombinatorialClass is deprecated. Please use the
FiniteEnumeratedSets category. For an example, see:
sage: C = FiniteEnumeratedSets().example()
sage: C??
File: /opt/sage/local/lib/python2.6/site-
packages/sage/categories/examples/finite_enumerated_sets.py
(sorry, we still have a lot of old code giving bad hints to the
newcomers)
- What about just implementing _an_element_? (no __iter__, no first).
Actually at this point DeBruijnSequences(alphabet, n) should just
be in the category FiniteSets(). However the later does not exist
yet, so Sets() will do.
- Rather than defining an attribute self.k_is_alphabet, I would setup
two attributes self.k and self.alphabet once for all in the
__init__. Then no more branching needs to be done later on.
Maybe rename the parameter k to alphabet (which is more explicit)
- DeBruijnSequence_evaluation -> DeBruijnSequences
No need for the DeBruijnSequence function at this point.
- The length of the sequence is known in advance, right? Then it
should be faster to allocate a bit list right away, and then fill
it up, rather than using append.
- In the long run, we might want to provide an iterator over the
sequence (once cython will support iterators)
- debruijn.py -> debruijn_sequence.py
- The speed difference is impressive :-) I don't know if we want to
mention it in the doc though (this may change in the future).
Cheers,
Nicolas
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/10530#comment:2>
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 post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
