Opened 6 years ago

Closed 6 years ago

#16358 closed defect (fixed)

Wrong answers of IncidenceStructure.automorphism_group()

Reported by: ncohen Owned by:
Priority: major Milestone: sage-6.3
Component: combinatorial designs Keywords:
Cc: vdelecroix, knsam, dimpase, brett Merged in:
Authors: Nathann Cohen Reviewers: Vincent Delecroix
Report Upstream: N/A Work issues:
Branch: d004ebc (Commits) Commit: d004ebcb357083ba06f435ffe75050e0f4ec70a1
Dependencies: Stopgaps:

Description

I would say that this was broken when #16237 was fixed. The cool news is that it did not make it inside of the stable release :-P

....

On the other hand all the doctests of automorphism_group were not changed, so it cannot be #16237. So it may have been wrong since the beginning ? O_o

Before this branch is applied : 

sage: BlockDesign(4,[[0,1,2,3],[1,2,3]],test=False).automorphism_group()
Permutation Group with generators [()]

After: 

sage: BlockDesign(4,[[0,1,2,3],[1,2,3]],test=False).automorphism_group()
Permutation Group with generators [(3,4), (2,3)]

Aaaaaaaand it turns out that writing this patch did not even force me to change any doctest.. So it was just bad luck that the automorphism groups used in the docstrings were EXACTLY the automorphism groups of the duals (like the Fano plane... :-P) :-P

Nathann

P.S. : this also adds some potentially useful keyword in is_block_design. That's how I found this bug.

Change History (9)

comment:1 Changed 6 years ago by ncohen

  • Branch set to u/ncohen/16358
  • Status changed from new to needs_review

comment:2 Changed 6 years ago by git

  • Commit set to 267a5f2a50bd43e6d247fcc7c4acb52c84b16b05

Branch pushed to git repo; I updated commit sha1. New commits:

​267a5f2trac 16358: Wrong answers of IncidenceStructure.automorphism_group()

comment:3 Changed 6 years ago by ncohen

  • Component changed from combinatorics to combinatorial designs

comment:4 Changed 6 years ago by ncohen

Hey guys... Sorry to bug you but this is a one-line bugfix, and in the meantime Sage really returns wrong results... :-/

Aaaaaaand #16367 is also a very simple improvement that does help a lot when playing with designs...

Nathann

comment:5 Changed 6 years ago by vdelecroix

Hi Nathann,

I did not like the doctests so I rewrote them... have a look u/vdelecroix/16358. And if you like, you can set to positive review.

Vincent

comment:6 Changed 6 years ago by ncohen

  • Branch changed from u/ncohen/16358 to u/vdelecroix/16358
  • Commit changed from 267a5f2a50bd43e6d247fcc7c4acb52c84b16b05 to d004ebcb357083ba06f435ffe75050e0f4ec70a1
  • Reviewers set to Vincent Delecroix
  • Status changed from needs_review to positive_review

>_<

I would love to get this PGL stuff some day... >_<

Thanks for your review !!!

Nathann

New commits:

​d004ebctrac #16358: cleaner doctests for automorphism_group

comment:7 Changed 6 years ago by vdelecroix

Please rebase #16367 and I will continue.

Vincent

comment:8 Changed 6 years ago by ncohen

Done.

Nathann

comment:9 Changed 6 years ago by vbraun

  • Branch changed from u/vdelecroix/16358 to d004ebcb357083ba06f435ffe75050e0f4ec70a1
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.