scheinerman / Permutations.jl

Permutations class for Julia.
Other
51 stars 14 forks source link

Bug in order of the trivial permutation #40

Closed laurentbartholdi closed 11 months ago

laurentbartholdi commented 11 months ago

A border case for order is not well handled:

julia> Permutation(Int[])
()

julia> order(ans)
ERROR: BoundsError: attempt to access 0-element BitVector at index [1]
Stacktrace:
 [1] throw_boundserror(A::BitVector, I::Tuple{Int64})
   @ Base ./abstractarray.jl:734
 [2] checkbounds
   @ Permutations ./abstractarray.jl:699 [inlined]
 [3] setindex!
   @ Permutations ./bitarray.jl:701 [inlined]
 [4] cycles(p::Permutation)
   @ Permutations ~/.julia/packages/Permutations/zIubD/src/Permutations.jl:205
 [5] order(p::Permutation)
   @ Permutations ~/.julia/packages/Permutations/zIubD/src/Permutations.jl:299
 [6] top-level scope
   @ REPL[15]:1
laurentbartholdi commented 11 months ago

Not a bug, but it would make sense to add a isone method to detect triviality.

scheinerman commented 11 months ago

Thanks @laurentbartholdi . For the first item you mentioned, I should have order return 1.

For the second comment, are you proposing that isone(p::Permutation) should return true exactly when p is an identity permutation? That would be easily done, but just want to be clear on your suggestion.

laurentbartholdi commented 11 months ago

Yes, exactly! a permutation p can be multiplied with another one, and isone(p) precisely when p*q=q, so it makes perfect sense to have the identity permutation satisfy isone. Also isone(p) if and only if isone(matrix(p)).

scheinerman commented 11 months ago

Thanks again. Changes implemented and should be visible soon as version 0.4.18. Do you think it would make sense to also add a one method? That is one(p) would return the identity permutation of the same length as p? Not sure this is needed.

scheinerman commented 11 months ago

Having a tad of trouble registering ...

scheinerman commented 11 months ago

Having a tad of trouble registering ...

Problem solved. New version is registered.