sympy.combinatorics: rename Permutation to Perm, remove checks from constructor?

[Aleksandar Makelov]

So it's been suggested ( see the discussion of https://github.com/sympy/sympy/pull/1319
) that we get rid of the slightly long Permutation and replace it with
the shorter Perm in sympy/combinatorics/permutations.py. Keeping with
this, we should also rename PermutationGroup to PermGroup in sympy/
combinatorics/perm_groups.py. It sounds like a good idea to me. Are
there any objections to that?

Also, there was a discussion about the checks that are performed every
time we construct a Permutation object - whether all numbers from 0 to
n-1 are present and the arguments provided are the way they should be.
They tend to greatly slow down the construction of a Permutation
object; on the other hand, it might be helpful to keep them in order
to quickly spot if you're doing something stupid. And there are fast
factory functions for constructing permutations that skip these
checks, like new_from_array_form. But it seems that users might also
want to skip the checks if they know what they're doing and are
working with permutations of sizes in the millions.

So, do you have any suggestions about what we should do, keep the
checks, remove them, or something in between?

[Sergiu Ivanov]

On Sat, Jun 9, 2012 at 11:55 PM, Aleksandar Makelov
<amak...@college.harvard.edu> wrote:
>
> Also, there was a discussion about the checks that are performed every
> time we construct a Permutation object - whether all numbers from 0 to
> n-1 are present and the arguments provided are the way they should be.
> They tend to greatly slow down the construction of a Permutation
> object; on the other hand, it might be helpful to keep them in order
> to quickly spot if you're doing something stupid. And there are fast
> factory functions for constructing permutations that skip these
> checks, like new_from_array_form. But it seems that users might also
> want to skip the checks if they know what they're doing and are
> working with permutations of sizes in the millions.

I guess giving the users the possibility to control this by setting a
flag somewhere is the most canonical approach, that also works greatly
in a wide range of circumstances.

Sergiu

[krastano...@gmail.com]

About renaming: It seems ok and somebody mentioned that Perm is the
name used by other established software packages.

About checks: Keep in mind that sympy objects are expected to be rebuildable.

`type(your_object)(your_object.args)` is expected to be equal to
`your_object`. So if Perm does the checks they will be redone
redundantly on each rebuild.

Then one can considered the addition of keyword arg to Perm
(check=False/True). I dislike this idea, because it changes the way a
Basic object is created (again in a clash with the idea of
rebuildability).

I would prefer the creation of `safe_perm` function that uses a few
`assert` statements for the checks and then it just calls `Perm` which
does not do any checks.

Another reason that I prefer no checks in Perm is that it follows the
"consenting adults" style.

[David Joyner]

On Sat, Jun 9, 2012 at 4:55 PM, Aleksandar Makelov
<amak...@college.harvard.edu> wrote:
> So it's been suggested ( see the discussion of https://github.com/sympy/sympy/pull/1319
> ) that we get rid of the slightly long Permutation and replace it with
> the shorter Perm in sympy/combinatorics/permutations.py. Keeping with
> this, we should also rename PermutationGroup to PermGroup in sympy/
> combinatorics/perm_groups.py. It sounds like a good idea to me. Are
> there any objections to that?

Is there an objection to using both?
It's a simple matter of adding one line to the code.

>
> Also, there was a discussion about the checks that are performed every
> time we construct a Permutation object - whether all numbers from 0 to
> n-1 are present and the arguments provided are the way they should be.
> They tend to greatly slow down the construction of a Permutation
> object; on the other hand, it might be helpful to keep them in order
> to quickly spot if you're doing something stupid. And there are fast
> factory functions for constructing permutations that skip these
> checks, like new_from_array_form. But it seems that users might also
> want to skip the checks if they know what they're doing and are
> working with permutations of sizes in the millions.
>
> So, do you have any suggestions about what we should do, keep the
> checks, remove them, or something in between?
>

> --
> You received this message because you are subscribed to the Google Groups "sympy" group.
> To post to this group, send email to sy...@googlegroups.com.
> To unsubscribe from this group, send email to sympy+un...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/sympy?hl=en.
>

[Aaron Meurer]

Did you determine that this was expensive from actual benchmarks?  It doesn't seem to me like it would be that big of an issue. You can also probably find clever ways to do it that are more efficient than the niave methods. 

Then again, I can see how a lot of rebuilds would affect this, so maybe a _from_rawargs would be worthwhile for internal algorithms that know that they don't need the checks. 

Aaron Meurer

--
Sent from my iPad.

[David Joyner]

On Sat, Jun 9, 2012 at 7:20 PM, Aaron Meurer <asme...@gmail.com> wrote:
> Did you determine that this was expensive from actual benchmarks?  It

I'm really just guessing an extra namespace lookup won't affect things much,
but I don't know. I think it is just a matter if you care if the namespace
is slightly more polluted or not. I'm fine with both, or either one.

[Joachim Durchholz]

Am 09.06.2012 22:55, schrieb Aleksandar Makelov:
> So it's been suggested ( see the discussion of https://github.com/sympy/sympy/pull/1319
> ) that we get rid of the slightly long Permutation and replace it with
> the shorter Perm in sympy/combinatorics/permutations.py. Keeping with
> this, we should also rename PermutationGroup to PermGroup in sympy/
> combinatorics/perm_groups.py. It sounds like a good idea to me. Are
> there any objections to that?

I'm having difficulties with that - PermGroup could also mean a group of
permanent objects. My knee-jerk reaction was "that's awful" for that
reasons.
However, that all is because I'm coming from a systems programming
background, where "permanent" is an important and central concept in
multiple contexts. I'm not sure how much of that would apply for
someobdy with a mainly mathematical background.

> Also, there was a discussion about the checks that are performed every
> time we construct a Permutation object - whether all numbers from 0 to
> n-1 are present and the arguments provided are the way they should be.

> [...]

>
> So, do you have any suggestions about what we should do, keep the
> checks, remove them, or something in between?

I'd do the checks by default but offer ways (clearly marked as such)
that come without the checks.

Approaches that I can see (including those that I have seen in the pull
request discussion for completeness):

1) Offer a set of "safe" (checking) and a set of "unsafe" (not checking)
operations. Unsafe operations are for users that know what they're
doing, and for SymPy code that knows what it's doing (e.g. when passing
the output from a Permutation-generating function to the input of a
Permutation-taking function - the latter could be the unsafe variant.)

2) Remove the checking altogether and make it an optional thing that
people may call to verify their inputs if they want it.

3) Have Permutations carry a "safe" flag. Reset it on any change that's
not known to keep the Permutation properties. Set it on any change that
establishes Permutation properties.

4) As (3), but don't use a flag, use the type. A "Sequence" (maybe
somebody has a more precise name) is a finite list of integers. A
"Permutation" is a "Sequence" where every integer between start and end
is present exactly once.
This approach would make it possible to reuse some permutation code for
"Sequences". Cool or superfluous, I don't know :-)
I'm not sure whether Python allows to change the class of an object
after it was created. If not, every change between Permutation and
Sequence would involve object creation; this may or may not be a problem
in practice, depending on whether the Permutation code is creating new
objects anyway, and depending on the actual object creation overhead.

Just my 2c.

[Aleksandar Makelov]

Hi again,

Based on the discussion I can suggest the following concrete changes:
- for now, use both Perm and Permutation as names for the class defining a permutation
- similarly, use both PermGroup and PermutationGroup as names for the class defining a permutation group
- remove the checks from Perm, and make a *function* PermS (the name is deliberately chosen to be as close as Perm) that performs the checks and then constructs a permutation object using Perm (the name comes from Permutation Safe).

What do you guys think?

[krastano...@gmail.com]

> - remove the checks from Perm, and make a *function* PermS (the name is
> deliberately chosen to be as close as Perm) that performs the checks and
> then constructs a permutation object using Perm (the name comes from
> Permutation Safe).

I am in favor of this. `Basic` subclasses should exist for
mathematical concepts, not for user convenience (especially when we
take the need for reconstruction of these classes and their expression
trees into account). A helper function is a much better solution than
some magic done in the `__new__` method.

[mario]

+1

[David Joyner]

+1

>
> --
> You received this message because you are subscribed to the Google Groups
> "sympy" group.

> To view this discussion on the web visit
> https://groups.google.com/d/msg/sympy/-/eHdNa_c2shoJ.

[Ronan Lamy]

Le mercredi 13 juin 2012 à 02:06 -0700, Aleksandar Makelov a écrit :
> Hi again,
>
> Based on the discussion I can suggest the following concrete changes:
> - for now, use both Perm and Permutation as names for the class
> defining a permutation
> - similarly, use both PermGroup and PermutationGroup as names for the
> class defining a permutation group

"Perm" is really a bad name that doesn't suggest anything to do with
permutations. "PermGroup" is less bad but isn't that much shorter
either. In any case, having more than one way to do it is bad, as a
general principle, so sympy code should only use the explicit
alternatives.

> - remove the checks from Perm, and make a *function* PermS (the name
> is deliberately chosen to be as close as Perm) that performs the
> checks and then constructs a permutation object using Perm (the name
> comes from Permutation Safe).

Urgh, no! The design is OK, though not terribly consistent with the rest
of sympy where users need to directly call class constructors all the
time, but the naming seems designed to maximise user confusion. Casual
users shouldn't need to know about the concept of permutation
"safeness", nor have to remember a cryptic abbreviation for it, and
functions names (conventionally lowercase) shouldn't make them look like
classes.

[David Joyner]

On Wed, Jun 13, 2012 at 8:31 AM, Ronan Lamy <ronan...@gmail.com> wrote:
> Le mercredi 13 juin 2012 à 02:06 -0700, Aleksandar Makelov a écrit :
>> Hi again,
>>

...

>
>> - remove the checks from Perm, and make a *function* PermS (the name
>> is deliberately chosen to be as close as Perm) that performs the
>> checks and then constructs a permutation object using Perm (the name
>> comes from Permutation Safe).
>
> Urgh, no! The design is OK, though not terribly consistent with the rest
> of sympy where users need to directly call class constructors all the
> time, but the naming seems designed to maximise user confusion. Casual
> users shouldn't need to know about the concept of permutation
> "safeness", nor have to remember a cryptic abbreviation for it, and
> functions names (conventionally lowercase) shouldn't make them look like
> classes.

I'm a little confused. Is PermS a user function? I thought it was internal.
If it is internal, and not a function for the casual user, then it
might be renamed _PermS, so it doesn't pop up in tab completion.

Also, a line or two on the abbreviation conventions in the top-level docstring
might help the casual user.

>
> --
> You received this message because you are subscribed to the Google Groups "sympy" group.

[mario]

On Wednesday, June 13, 2012 2:44:58 PM UTC+2, David Joyner wrote:

On Wed, Jun 13, 2012 at 8:31 AM, Ronan Lamy  wrote:
> Le mercredi 13 juin 2012 à 02:06 -0700, Aleksandar Makelov a écrit :
>> Hi again,
>>

...

>
>> - remove the checks from Perm, and make a *function* PermS (the name
>> is deliberately chosen to be as close as Perm) that performs the
>> checks and then constructs a permutation object using Perm (the name
>> comes from Permutation Safe).
>
> Urgh, no! The design is OK, though not terribly consistent with the rest
> of sympy where users need to directly call class constructors all the
> time, but the naming seems designed to maximise user confusion. Casual
> users shouldn't need to know about the concept of permutation
> "safeness", nor have to remember a cryptic abbreviation for it, and
> functions names (conventionally lowercase) shouldn't make them look like
> classes.


I'm a little confused. Is PermS a user function? I thought it was internal.
If it is internal, and not a function for the casual user, then it
might be renamed _PermS, so it doesn't pop up in tab completion.

Using Perm in Permutation is a bit ugly, but it is endorsed by Knuth
(quote which I found in http://www.xact.es/xPerm/index.html)
''Following a suggestion of Vaughan Pratt, we adopt the convention that perm = permutation, perhaps thereby saving millions of syllables in future research.''

I  can accept that PermS be used to create safely permutations, along  with Perm which creates them without checking, although I would then use only PermS, since I care about the checks; it is a place less to check if I have a bug.

If I need speed I use _new_from_array_form, which is faster than the proposed Perm. I am strongly against  _PermS, which is not convenient to type.