Ticket #3719 (closed defect: fixed)

Opened 4 months ago

Last modified 3 months ago

[with spkg, patch, positive review] bug in group cohomology

Reported by: wdj Assigned to: tbd
Priority: major Milestone: sage-3.1.2
Component: algebra Keywords:
Cc: AlexGhitza

Description

As reported by Ursula Whitcher 

sage: G = SymmetricGroup(4)
sage: G.cohomology(1,2)

yields an error. The problem is a bug in the current version of the GAP package HAP, version 1.8.5. The latest version 1.8.7 but there the bug still exists 

gap> G := SymmetricGroup(4);
Sym( [ 1 .. 4 ] )
gap> GroupCohomology(G,1); ## an improvement over 1.8.5
[  ]
gap> GroupCohomology(G,1,2);
List Element: <position> must be a positive integer (not a integer) at
if IsInt( C!.fpIntHom[n] )  then
...

Attachments

10128.patch (2.1 kB) - added by wdj on 08/03/2008 07:48:09 AM.
docstring addition patch based on 3.1.alpha0
10129.patch (4.0 kB) - added by wdj on 08/03/2008 09:02:48 PM.
based on 3.1.alpha0 and probably the previous patch

Change History

07/29/2008 10:16:46 AM changed by mabshoff

  • milestone set to sage-3.1.1.

08/02/2008 07:35:14 PM changed by wdj

I posted a new gap_packages archive to http://sage.math.washington.edu/home/wdj/patches/gap_packages-4.4.10_6.spkg

08/02/2008 07:38:32 PM changed by wdj

Sorry, meant to add that this passes sage -testall on amd64 hardy heron and includes hap1.8.8 which fixes the problem reported above. 

sage: gap.eval('LoadPackage("hap")')
'true'
sage: G = SymmetricGroup(4)
sage: G.cohomology(1,2)
Multiplicative Abelian Group isomorphic to C2

08/03/2008 01:13:34 AM changed by mabshoff

  • summary changed from bug in group cohomology to [with spkg, needs review] bug in group cohomology.

David,

please also add an optional doctest that verifies that the issue has been fixed.

Cheers,

Michael

(follow-up: ↓ 6 ) 08/03/2008 05:13:28 AM changed by wdj

Michael: You mean, supply a patch to permgroup.py with this addition to the docstring to the group_cohomology method for a new trac ticket?

(in reply to: ↑ 5 ) 08/03/2008 06:51:02 AM changed by mabshoff

Replying to wdj:

Michael: You mean, supply a patch to permgroup.py with this addition to the docstring to the group_cohomology method for a new trac ticket?

Yes, it should be an optional doctest. I am somewhat concerned that the author of the GAP package just rereleased the package with the bug fix instead of incrementing the release number. Since various packaging efforts are under way I could imagine something like this not getting fixed upstream in other distributions, so the doctest when run with optional flags would show that the problem remains unfixed.

It is still my plan to used optional testing in the future at least for the build on sage.math per default.

Cheers,

Michael

08/03/2008 07:48:09 AM changed by wdj

  • attachment 10128.patch added.

docstring addition patch based on 3.1.alpha0

08/03/2008 07:54:28 AM changed by wdj

Okay, I just attached the patch you requested to this ticket. (I wasn't sure if it needed a new ticket or not.) It passes sage -t but it dawned on me afterwards that sage -t would not test for optional docstring additions. Anyway, hope this is what you were looking for.

BTW, I am one of the webmasters for GAP (hence involved wityh package updates) and you can be sure that hap 1.8.8 will definitely get applied upstream, probably in the next week or so.

08/03/2008 10:48:28 AM changed by was

Okay, I just attached the patch you requested to this ticket. (I wasn't sure if it needed a new ticket or not.) It passes sage -t but it dawned on me afterwards that sage -t would not test for optional docstring additions.

Use 

  sage -t --optional

08/03/2008 09:02:48 PM changed by wdj

  • attachment 10129.patch added.

based on 3.1.alpha0 and probably the previous patch

08/03/2008 09:08:31 PM changed by wdj

Okay, the attached passes sage -t --optional I had to make some changes to the code. For some reason, it seems gap packages were not loading for me properly (eg, gap.eval('LoadPackage?("hap")') gave a traceback the first time but loaded it the second time...). That was fixed with some try/except trickery which seems harmless even if packages do load as they should. Also, GAP's RequirePackage? is actually being deprecated, so I replaced them with the preferred LoadPackage?. Finally, Molien series do not require an optional package, so I removed some comments in the docstring for that method. Hope this is okay now.

08/26/2008 04:04:07 AM changed by AlexGhitza

  • cc set to AlexGhitza.

08/26/2008 11:44:47 PM changed by AlexGhitza

  • summary changed from [with spkg, needs review] bug in group cohomology to [with spkg and two patches, positive review] bug in group cohomology.

Looks good. The new spkg fixes the problem that was reported, and the few things that broke in the upgrade are fixed by the two patches.

08/27/2008 01:08:18 AM changed by mabshoff

  • status changed from new to closed.
  • resolution set to fixed.
  • summary changed from [with spkg and two patches, positive review] bug in group cohomology to [with spkg, patch, positive review] bug in group cohomology.

Merged both patches in Sage 3.1.2.alpha1.

David,

I added an hg repo to the spkg (but kept the p6 patchlevel). Please base future spkgs on the spkg

http://sage.math.washington.edu/home/mabshoff/release-cycles-3.1.2/alpha1/gap_packages-4.4.10_6.spkg

Cheers,

Michael