#15456: fix bug in has_right/left_descents in Weyl group code
-------------------------+-------------------------------------------------
Reporter: | Owner:
zabrocki | Status: needs_review
Type: | Milestone: sage-6.2
defect | Resolution:
Priority: major | Merged in:
Component: | Reviewers:
combinatorics | Work issues:
Keywords: | Commit:
Authors: | bef966f7e6f1ce42da5578c8da4df8d5ebdf9f9b
Frédéric Chapoton | Stopgaps:
Report Upstream: N/A |
Branch: |
u/chapoton/15456 |
Dependencies: |
-------------------------+-------------------------------------------------
Comment (by tscrim):
I don't like having the warning saying "don't override this", especially
since I don't see a good reason for doing this (and there's no "final"
semantic in python). I am in favor of saying we must implement either
`has_left_descent()` or `has_right_descent()`. However I still think a
better fix will be for `has_left/right_descent()` to call `has_descent()`
by default.
Anyways, I've put my working version on trac at `u/tscrim/15456`. I also
gave default implementations of `has_left/right_descent()` to
`weyl_group.py` for speed. Although now that I've done the implementation,
I'm less convinced of my suggestion. Perhaps what would be best is some
category magic. We test upon creation of a Coxeter group to see if any of
the methods have been implemented. Thus we do the following:
- If `has_descent()` has been implemented, give default `has_*_descent()`
call `has_descent()` with the appropriate args for those not implemented.
- If none were implemented, error out on construction.
- Otherwise set it up as we have it now.
Maybe this is heavy-handed?
Nevertheless, here are some timings. I'm using the example from the ticket
description. Baseline:
{{{
sage: %timeit w.has_descent(3, side="left")
1000 loops, best of 3: 860 us per loop
sage: %timeit w.has_descent(3, side="left")
1000 loops, best of 3: 820 us per loop
sage: %timeit w.has_descent(3, side="right")
1000 loops, best of 3: 200 us per loop
sage: %timeit w.has_descent(3, side="right")
1000 loops, best of 3: 209 us per loop
}}}
With Frederic's branch:
{{{
sage: %timeit w.has_descent(3, side="left")
1000 loops, best of 3: 987 us per loop
sage: %timeit w.has_left_descent(3)
1000 loops, best of 3: 929 us per loop
sage: %timeit w.has_descent(3, side="right")
1000 loops, best of 3: 204 us per loop
sage: %timeit w.has_right_descent(3)
1000 loops, best of 3: 215 us per loop
}}}
With my branch:
{{{
sage: %timeit w.has_descent(3, side="left")
1000 loops, best of 3: 867 us per loop
sage: %timeit w.has_left_descent(3)
1000 loops, best of 3: 902 us per loop
sage: %timeit w.has_descent(3, side="right")
1000 loops, best of 3: 200 us per loop
sage: %timeit w.has_right_descent(3)
1000 loops, best of 3: 191 us per loop
}}}
--
Ticket URL: <http://trac.sagemath.org/ticket/15456#comment:20>
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.
