Added K[G]-Algebras package #3685

danielk011 · 2025-03-09T00:48:30Z

No description provided.

mahrud

Thanks for your contribution! I left a few comments and feedback below.

There is also more information here: https://github.com/Macaulay2/M2/wiki/Package-Writing-Style-Guide

M2/Macaulay2/packages/KGAlgebras.m2

mahrud · 2025-03-09T01:07:15Z

M2/Macaulay2/packages/KGAlgebras.m2

+		for j in 0 .. n do( 
+			productInv = position(matrixElts,M -> M==matrixElts#i * matrixElts#j);
+			H= (FA_i)*(FA_j)-(FA_productInv);
+			multTableIdealGens=append(multTableIdealGens,H);


Appending to a list in Macaulay2 allocates memory for and creates a new list, which can be slow depending on the size of n. You might want to consider using a MutableList instead, and adding to it using the (admittedly funny looking) syntax:

multTableIdealGens##multTableIdealGens = H

I think if the mutable list starts out empty, then we'll still end up allocating a bunch of new lists along the way as it grows, right? So it would probably be good to do something like this first since we know how long it will be:

mutlTableIdealsGens = new MutableList from (n+1)^2:null

Thank you for the suggestion. I'm getting errors from trying to create an ideal from a mutable list. Is there a nice way to generate an ideal from a mutable list that is officially supported?

You can call toList on it first, e.g.:

i1 : R = QQ[x]; i2 : ideal toList new MutableList from {x} o2 = ideal x o2 : Ideal of R

mahrud · 2025-03-09T01:08:27Z

M2/Macaulay2/packages/KGAlgebras.m2

+			NP = kernel MPP;
+			yp = (gens NP)_0;
+			M2 = matrix apply(H,z -> z*yp);
+			print(rank M2);


Was this print intentional? If it's for debugging purposes could make it conditional on value of debugLevel.

mahrud · 2025-03-09T01:09:39Z

M2/Macaulay2/packages/KGAlgebras.m2

+    )
+export {"KGAlg","meataxe", "SnGroup"}
+
+KGAlgebra = new Type of FreeAlgebraQuotient;


You probably also need to export the symbol KGAlgebra.

mahrud · 2025-03-09T01:11:44Z

M2/Macaulay2/packages/KGAlgebras.m2

+	local p;
+	local L;
+	local M;
+	local MP;
+	local y;
+	local m;
+	local q;
+	local Mp;
+	local M1;
+	local MPP;
+	local H;
+	local N;
+	local n;
+	local M2;
+	local NP;
+	local yp;	


Same comment as above about local symbols and :=.

mahrud · 2025-03-09T01:13:30Z

M2/Macaulay2/packages/KGAlgebras.m2

+				return false;
+			);--Checks that the module generated by M1 is not a strict submodule of KG
+
+			H = apply(group G, z -> transpose(z)); --Generate list of group elements' matrices but transposed


This is fine as is, but I wanted to point out this shorthand syntax:

Suggested change

H = apply(group G, z -> transpose(z)); --Generate list of group elements' matrices but transposed

H = apply(group G, transpose); --Generate list of group elements' matrices but transposed

mahrud · 2025-03-09T01:15:57Z

M2/Macaulay2/packages/KGAlgebras.m2

+--True implies irreducible, false implies reducible
+meataxe = method()
+meataxe(FiniteGroupAction, FreeAlgebraQuotient,Ring) := (G,S,K) -> (


I'm a little confused, the standard Meataxe algorithm returns the indecomposable summands, no?
If your implementation only solves the decision problem, it could be called something like isIndecomposable or isIrreducible.

mahrud · 2025-03-09T01:17:04Z

M2/Macaulay2/packages/KGAlgebras.m2

+    {Name => "Toshi Mowery", Email => "[email protected]"}
+    },
+    Headline => "A package for KG-algebras",
+    Keywords => {"KG-algebra"},


Keywords should be one (or more) of the categories on this page. I would recommend "Group Theory" instead.

mahrud · 2025-03-09T03:25:30Z

One more thought: I would consider naming your main type GroupAlgebra (or GroupRing) and the main constructor groupAlgebra (or groupRing). This is more closely aligned with the naming conventions.

M2/Macaulay2/packages/KGAlgebras.m2

d-torrance · 2025-03-11T20:41:28Z

M2/Macaulay2/packages/KGAlgebras.m2

+    groupElts := g_0 .. g_n; -- group elements
+    FA := freeAlgebra(K,groupElts);
+    use FA; -- make FA our working ring
+    multTableIdealGens := new MutableList;


See the suggestion above. Starting with an empty mutable list isn't much better than the old append implementation since we'll end up having to reallocate everything every time we grow it by one more element. But we know the final length, so we can start out with a mutable list of the correct length and not have to worry about it ever growing:

multTableIdealGens := new MutableList from (n + 1)^2:null

Another option would be to avoid mutable lists entirely and use apply:

multTableIdealGens = apply((n + 1)^2, k -> ( (i, j) := quotientRemainder(k, n + 1); ... H)

For the suggested edits I've noticed that multTableIdealGens##multTableIdealGens = H adds new elements to the mutable list, growing its size instead of editing the empty entries. Am I missing something?

Oops, yeah I missed that! multTableIdealGens##multTableIdealGens = H will always grow the list, even if we've allocated a bunch of elements at the beginning since #multTableIdealGens will always point one past the last index!

Instead, something like multTableIdealGens#((n + 1)*i + j) = H should work.

d-torrance · 2025-03-11T20:42:14Z

M2/Macaulay2/packages/KGAlgebras.m2

-    Reload=>true,
+    Headline => "A package for group algebras",
+    Keywords => {"Group Theory"},
+    DebuggingMode => true,


DebuggingMode should either be false or (even better) removed, since false is the default.

d-torrance · 2025-03-11T20:43:34Z

M2/Macaulay2/packages/KGAlgebras.m2

+            multTableIdealGens##multTableIdealGens = H;
+        );
+    );
+    print multTableIdealGens;


Did you mean to print this? I'd either remove it or swap it with something like if debugLevel > something then print ....

d-torrance · 2025-03-11T20:48:22Z

M2/Macaulay2/packages/KGAlgebras.m2

+        for x in (flatten entries L#0) do(
+            n := index(x);
+            M = M + (group G)#n * sub((flatten entries L#1)#m,K);
+            m = m+1    


This is totally optional, but if you're interested, you can use augmented assignment here:

M += (group G)#n * ... m += 1

It also works for other binary operators like *, so line 74 could become:

Mp *= (apply(...

d-torrance · 2025-03-11T21:27:30Z

M2/Macaulay2/packages/KGAlgebras.m2

+    Key
+        meataxe        
+    Headline
+        Boolean function implementing the meataxe algorithm determining the irreducibility of a finite group representation of a finite dimensional vector space. If the module is irreducible, returns true. Otherwise, the method returns a nontrivial submodule. 


The builds are failing with the following error:

../../../../../Macaulay2/m2/document.m2:598:32:(1):[72]: error: document: documentation headlines must be less than 100 characters long: "Boolean function implementing the meataxe algorithm determining the irreducibility of a finite group representation of a finite dimensional vector space. If the module is irreducible, returns true. Otherwise, the method returns a nontrivial submodule."

In general, headlines should just be a few words. Most of this stuff can get moved elsewhere in the documentation node.

d-torrance · 2025-03-12T12:28:56Z

M2/Macaulay2/packages/KGAlgebras.m2

+    {Name => "Moses Samuelson-Lynn", Email => "[email protected]"},
+    {Name => "Toshi Mowery", Email => "[email protected]"}
+    },
+    Headline => "A package for group algebras",


The tests are failing because of the redundant "package for" in the headline:

/Users/runner/work/M2/M2/M2/Macaulay2/tests/normal/release-checklist.m2:1:1 error: -- -- "^[A-Z]", -- headlines should not be capitalized, but hard to check -- "\\.$", -- headlines should not end with a period -- "\\n", -- headlines should not contain line breaks -- -- redundant clauses for package headlines: -- "(T|t)his (P|p)ackage", -- "(P|p)ackage (for|on|to)"}, opts.Headline)); -- -- i10 : if #pkgs > 0 then error("packages whose headlines doesn't follow guidelines:", newline, -- toString netList(toList \ pairs applyValues(pkgs, opts -> opts.Headline))) -- stdio:23:23:(3): error: packages whose headlines doesn't follow guidelines: -- +----------+----------------------------+ -- |KGAlgebras|A package for group algebras| -- +----------+----------------------------+ -- stdio:1:5:(3): error: test(s) #252 of package Core failed.

d-torrance · 2025-03-12T12:33:11Z

M2/Macaulay2/packages/KGAlgebras.m2

+                print(rank M2);
+            );
+            if rank M2 != rank (H#0) then (
+                return image M2;


Sometimes this function returns a boolean and sometimes it returns a module. That seems confusing for the caller -- how do they know what type to expect?

Would it make sense to return both as a sequence? So the caller could do something like:

(isIrreducible, M) = meataxe(G,S,K) if isIrreducible then ...

I have another implementation of the meataxe for modules and we return a list of the indecomposable pieces. So if the input M is indecomposable, the ouput is just {M}. You could try this instead.

d-torrance · 2025-04-19T17:15:03Z

M2/Macaulay2/packages/KGAlgebras.m2

+            The underlying field of the other parameters.
+    Outputs
+        T:Boolean
+            Returns true if the Meataxe algorithm states that the K[G]-module is irreducible. Returns a nontrivial submodule if the module is reducible. 


The documentation should be updated since this doesn't return a boolean any more.

Also, now that a module is return in both cases, how do we know if it's irreducible?

seangrate · 2025-05-05T20:26:19Z

One more thought: I would consider naming your main type GroupAlgebra (or GroupRing) and the main constructor groupAlgebra (or groupRing). This is more closely aligned with the naming conventions.

I'd also consider renaming the package "GroupAlgebras" or something along those lines.

danielk011 added 2 commits March 8, 2025 18:44

added KG Algebras package

5df1cb5

Added KG-Algebras

3d817bd

mahrud reviewed Mar 9, 2025

View reviewed changes

d-torrance changed the base branch from master to development March 9, 2025 01:49

d-torrance requested changes Mar 9, 2025

View reviewed changes

M2/Macaulay2/packages/KGAlgebras.m2 Outdated Show resolved Hide resolved

Fixed implementation of meataxe algorithm

3c3b027

danielk011 requested a review from d-torrance March 11, 2025 20:25

d-torrance requested changes Mar 11, 2025

View reviewed changes

Fixed package error and made list more efficient

e67358c

d-torrance reviewed Mar 12, 2025

View reviewed changes

d-torrance added the waiting for update label Mar 14, 2025

danielk011 and others added 2 commits March 25, 2025 20:45

Fixed package headline redundancy, made function return type consistent

6024bd2

Merge branch 'development' into master

dcbad60

d-torrance requested changes Apr 19, 2025

View reviewed changes

	H = apply(group G, z -> transpose(z)); --Generate list of group elements' matrices but transposed
	H = apply(group G, transpose); --Generate list of group elements' matrices but transposed

Added K[G]-Algebras package #3685

Are you sure you want to change the base?

Added K[G]-Algebras package #3685

Uh oh!

Conversation

danielk011 commented Mar 9, 2025

Uh oh!

mahrud left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

d-torrance Mar 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mahrud commented Mar 9, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seangrate commented May 5, 2025

Uh oh!

Uh oh!

mahrud left a comment •

edited

Loading

d-torrance Mar 9, 2025 •

edited

Loading