#17979: Reimplementation of IntegerListsLex
-------------------------------------+-------------------------------------
Reporter: aschilling | Owner:
Type: defect | Status: needs_review
Priority: blocker | Milestone: sage-6.6
Component: combinatorics | Resolution:
Keywords: days64 | Merged in:
Authors: Bryan Gillespie, | Reviewers: Nathann Cohen, Jeroen
Anne Schilling, Nicolas M. Thiery | Demeyer
Report Upstream: N/A | Work issues:
Branch: | Commit:
public/ticket/17979 | f73c43fd4423675f9ce279da9bb929a44db31483
Dependencies: | Stopgaps:
-------------------------------------+-------------------------------------
Comment (by jdemeyer):
Replying to [comment:178 nthiery]:
> Ok, I believe all the points that have been raised have been catered
> for (but some may have slipped through in this long thread). So this
> is up for formal review.
Let me remind everybody here that #17920 is ''also'' up for formal review.
Comparing both approaches (I am of course not completely unbiased, but
I'll really try to be objective):
* '''Bugs''': neither #17979 nor #17920 have obvious bugs (although I find
[comment:166] dubious at least), both fix #17548. I assume both pass
doctests.
* '''Features''': #17979 tries to replicate the old behaviour as closely
as possible, #17920 generalizes certain conditions (e.g. allowing negative
numbers; allowing iteration in non-invlex order) but also adds a few
restrictions (e.g. not allowing an iterable for `n`).
* '''Interface with rest of Sage''': #17920 only uses the new
`IntegerLists` code for `IntegerListsLex`, `Partitions` and `Compositions`
(in the other places in Sage where `IntegerListsLex` was used, I didn't
find any bugs so I left those). This ticket #17979 replaces the old code
in almost all cases (except partially in `IntegerVectors`).
* '''Speed''': for simple examples, #17979 is faster. Both implementations
have cases where they behave pathologically: for #17979 this is for
example {{{IntegerListsLex(10^10, length=2, min_slope=0, max_slope=0)}}},
while #17920 behaves badly if the length gets large.
* '''Extensibility''': because #17920 is based on polyhedra, it will be
much easier to generalize the code (it already does some things more
general than #17979). Also, if Sage ever gets better `Polyhedron` code, it
will make #17920 faster for free.
--
Ticket URL: <http://trac.sagemath.org/ticket/17979#comment:181>
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/d/optout.
