Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

note added about Cycle and Permutation application of non-disjoint cycles #12510

Merged
merged 1 commit into from Apr 15, 2017

Conversation

smichr
Copy link
Member

@smichr smichr commented Apr 8, 2017

Old notes hidden in the comments of this OP.

No change has been made other than to put a note of caution in the docstring of permutation.

closes #12487

@smichr
Copy link
Member Author

smichr commented Apr 8, 2017

ping @siefkenj

@siefkenj
Copy link
Contributor

siefkenj commented Apr 8, 2017

I'm finding this pretty hard to follow...The functionality I need is the following: A call Permuation(some_list).list() == some_list provided that some_list is a permutation and a call Permutation(list_of_tuples).list() applies the cycles from left-to-right. Maybe I am misreading the code, but it seems like Cycle(some_list) applies some_list as a cycle, not as an explicitly specified permutation as Permutation would.

@smichr
Copy link
Member Author

smichr commented Apr 8, 2017

Here's a demo:

>>> from sympy.combinatorics import *
>>> Permutation([1,2,3,5,0,4]).list()
[1, 2, 3, 5, 0, 4]
>>> l2r = Permutation([1,2], [2,3])  # multi-lists is NEW
>>> r2l = Permutation([[1,2], [2,3]])  # list of lists (old R to L evaluation)
>>> l2r == Permutation(2,3)(1,2)
True
>>> r2l == Permutation(1,2)(2,3)
True

@siefkenj
Copy link
Contributor

Okay. I still don't think this can be used for my purposes, though. If someone gives me a list of numbers, it is an explicit permutation, and if someone gives me a list of pairs, it is a list of swaps when evaluated left to right. So, if someone gives me w, I cannot do Permutation(w), nor can I do Permutation(*w), which isn't to comment on whether this is a worthwhile addition to the Permutation class, but it doesn't free me from going a bunch of manual argument checking.

@smichr
Copy link
Member Author

smichr commented Apr 10, 2017

but it doesn't free me from going a bunch of manual argument checking.

See point (2) in the OP. With that you could do Permutation(w, from_left=True) if the from_left were only used to handle the list of lists case. But I would prefer to just fix this via that pathway given in the second sentence of point 2. What do you think about that?

In the meanwhile, I think Permutation(w if not w or not is_sequence(w[0]) else list(reversed(w))) should work.

@jksuom
Copy link
Member

jksuom commented Apr 10, 2017

It is often confusing that there are two different ways of composing permutations. This is how the product of permutations is defined in SymPy:

Return the product a*b as a Permutation; the ith value is b(a(i)).

However, permutations are also mappings and, when considered as mappings, the composed mapping whose value at i is b(a(i)) is denoted b°a (with degree sign emulating the proper \circ sign).

Hence 'left to right' using one composition means 'right to left' with the other composition. So the ordering of cycles in this example

        >>> C((2, 3), (1, 2))
        (1 3 2)
        >>> C((1, 2), (2, 3))
        (1 2 3)

is opposite to that of composed permutations

>>> p = Permutation(Cycle(1, 2))
>>> q = Permutation(Cycle(2, 3))
>>> q*p  # ordered as in C((2, 3), (1, 2))
Permutation(1, 2, 3)
>>> p*q
Permutation(1, 3, 2)

@smichr
Copy link
Member Author

smichr commented Apr 10, 2017

Thanks, @jksuom . And if you have any feedback on what should or shouldn't be added here, I would appreciate it. We could disallow multiple-list input to Permutation so as not to create an ambiguous situation (the single-list instantiation of a permutation with a permutation vs the multi-list instantiation with 2 or more cycles). But I like the symmetry of being able to do L to R instantiation with both as Foo((a,b),(c,d)) and R to L as Foo(c,d)(a,b).

Currently we have entry options of Cycle(a,b)(c,d) (R to L) and Permutation(a,b)(c,d) (R to L), Permutation([(a,b), (c,d)]) (R to L, all indices must be present) and Permutation([a,c,b,d]) ([a,b,c,d] is the desired permutation and all indices must be present). The only way to do L to R evaluation of cycles is to reverse the cycles given in a list to Permutation.

My suggested addition is to add Cycle((a, b), (c, f)) as a L to R option. We could also make Cycle([(a, b), (c, f)]) match Permutation's behavior and do evaluation R to L. The other addition present in this PR --which could be removed-- is to allow Permutation((a,b), (c,d)) to mean evaluation from L to R...but then we need a way to tell whether (a,b,c,d) is a cycle or a permutation in Permutation((a,b,c,d)).

But I wonder if a method named reverse could be used to convert a permutation with cycles (a,b) then (c,d) into the form it would have if (c,d) were done first and then (a,b)? That wouldn't be too hard to do. But would it be useful?

@jksuom
Copy link
Member

jksuom commented Apr 11, 2017

I'm afraid that having two methods of composing permutations would lead to confusion and coding errors. The definition currently used in SymPy is prevalent in the literature, and it is the one that those working in this area are expecting. That is also the order in which permutations are returned by some matrix methods. The alternative order does not seem to be used in SymPy. Hence I would prefer keeping a single order as in calls like Cycle(a, b)(c, d) (where a, b, c and d need not be all different).

@smichr
Copy link
Member Author

smichr commented Apr 11, 2017

The definition currently used in SymPy is prevalent in the literature

Glad for that reassurance.

OK, then, perhaps just adding a docstring example of the * multiplication and the Cycle notation would suffice.

>>> a = Cycle(1, 2)
>>> b = Cycle(2, 3)
>>> Permutation(a)*Permutation(b) == Cycle(1,2)(2,3)
True

In this comment I already demonstrate how the result for @siefkenj 's case can be handled.

@smichr smichr closed this Apr 11, 2017
@smichr smichr reopened this Apr 11, 2017
@asmeurer
Copy link
Member

I'm afraid that having two methods of composing permutations would lead to confusion and coding errors. The definition currently used in SymPy is prevalent in the literature, and it is the one that those working in this area are expecting. That is also the order in which permutations are returned by some matrix methods. The alternative order does not seem to be used in SymPy. Hence I would prefer keeping a single order as in calls like Cycle(a, b)(c, d) (where a, b, c and d need not be all different).

I completely agree with this.

@smichr
Copy link
Member Author

smichr commented Apr 11, 2017

I will simply add a note in the docstring about the ordering issue and that it only matters when the cycles are not disjoint. Or maybe the docs would be a better place for this?

Here is a workaround for @siefkenj 👍

>>> def LRPerm(w):
...     from sympy.utilities.iterables import is_sequence
...     return Permutation(w if not (w and is_sequence(w[0])) else
...     Permutation.rmul(*[Permutation(*i) for i in w]))
>>> LRPerm(((1,2),(2,3)))  # cycle input
(123)
>>> Permutation(2,3)(1,2)
(123)
>>> LRPerm((1,2,0))  # actual-permutation input
(012)

@smichr
Copy link
Member Author

smichr commented Apr 13, 2017

If someone can see if this added note is clear, then this is done.

@smichr smichr changed the title Cycle and Permutation have option to process cycles from left to right note added about Cycle and Permutation application of non-disjoint cycles Apr 13, 2017
Reverse-order multiplication is available with the rmul
method. A helper function like this can give reverse
order application of cycles when a list of lists is given
or else provide the permutation as given by a list of ints:

>>> def LRPerm(w):
...    from sympy.utilities.iterables import is_sequence
...    return Permutation(w if not (w and is_sequence(w[0])) else
...    Permutation.rmul(*[Permutation(*i) for i in w]))
...
>>> Permutation(1,2)(2,3)
Permutation(1, 3, 2)
>>> Permutation([(1,2), (2,3)])
Permutation(1, 3, 2)

LRPerm will apply cycles from left to right:

>>> LRPerm([(1,2), (2,3)])
Permutation(1, 2, 3)

LRPerm gives the same result when a single list is given:

>>> LRPerm([0,1,3,2])==Permutation([0,1,3,2])
True
@smichr smichr merged commit d027546 into sympy:master Apr 15, 2017
@smichr smichr deleted the cycle branch April 15, 2017 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Permutation and Cycle as a callables have confusing behavior
4 participants