Opened 7 years ago
Closed 7 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 7 years ago by
- Branch set to u/ncohen/16358
- Status changed from new to needs_review
comment:2 Changed 7 years ago by
- Commit set to 267a5f2a50bd43e6d247fcc7c4acb52c84b16b05
comment:3 Changed 7 years ago by
- Component changed from combinatorics to combinatorial designs
comment:4 Changed 7 years ago by
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 7 years ago by
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 7 years ago by
- 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:
d004ebc | trac #16358: cleaner doctests for automorphism_group
|
comment:7 Changed 7 years ago by
Please rebase #16367 and I will continue.
Vincent
comment:8 Changed 7 years ago by
Done.
Nathann
comment:9 Changed 7 years ago by
- Branch changed from u/vdelecroix/16358 to d004ebcb357083ba06f435ffe75050e0f4ec70a1
- Resolution set to fixed
- Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits:
trac 16358: Wrong answers of IncidenceStructure.automorphism_group()