Skip to content

doc: fix IrreducibleModules description to say 'at most' dim#6337

Merged
fingolfin merged 1 commit intogap-system:masterfrom
mvanhorn:osc/6004-irreducible-modules-doc
Apr 23, 2026
Merged

doc: fix IrreducibleModules description to say 'at most' dim#6337
fingolfin merged 1 commit intogap-system:masterfrom
mvanhorn:osc/6004-irreducible-modules-doc

Conversation

@mvanhorn
Copy link
Copy Markdown
Contributor

Closes #6004.

Problem

The GAPDoc description for IrreducibleModules at lib/grpreps.gd:49-53 claimed the second return entry was:

a list of all irreducible modules of G over the field F in dimension dim

The actual behaviour is to return all irreducible constituents whose dimension is at most dim, same as the sibling operation AbsolutelyIrreducibleModules.

Evidence

Two independent confirmations in the same file / codebase:

  1. Sibling operation documents it correctly. AbsolutelyIrreducibleModules at lib/grpreps.gd:18-23 already says "can be realized over the finite field F and have dimension at most dim". Both operations share the same (G, F, dim) signature and are implemented side by side; the description should match.

  2. InstallOtherMethod comment confirms 0 means "no bound". lib/grpreps.gi:314 says:

    InstallOtherMethod(IrreducibleModules,"Supply no dimension limit",
      [IsGroup, IsField, ...], 0,
    function( G, F ) return IrreducibleModules(G,F,0); end);
    

    Only makes sense if dim=0 lifts the upper bound, not if it selects modules of dimension exactly zero.

Change

Two-line diff to lib/grpreps.gd:

-##  <A>G</A> over the field <A>F</A> in dimension <A>dim</A>, given as MeatAxe modules
+##  <A>G</A> over the field <A>F</A> of dimension at most <A>dim</A>,
+##  given as MeatAxe modules
 ##  (see&nbsp;<Ref Func="GModuleByMats" Label="for generators and a field"/>).
+##  Pass <C>0</C> for <A>dim</A> to impose no dimension bound.

The Pass 0 to impose no dimension bound sentence is a small addition that surfaces existing behaviour — previously only discoverable from the InstallOtherMethod comment — directly in the reference manual.

Docs only. No behaviour change.

@fingolfin fingolfin added the topic: documentation Issues and PRs related to documentation label Apr 20, 2026
@fingolfin
Copy link
Copy Markdown
Member

@mvanhorn hi there, thanks for your contribution.

As per our AI policy,

Any use of AI tools for preparing code, documentation, tests, commit messages, pull requests, issue comments, or reviews for this repository must be disclosed

@mvanhorn
Copy link
Copy Markdown
Contributor Author

@fingolfin Apologies for missing the disclosure up front. This PR was prepared with Claude Code (Anthropic's AI coding assistant): it helped me navigate the grpreps sources, confirm the "at most" semantics via the sibling operation and the InstallOtherMethod comment, and draft the two-line doc change. I reviewed the diff and PR body before opening.

Happy to add a disclosure line to the PR description, or to close if you would prefer a non-AI-assisted submission.

@fingolfin
Copy link
Copy Markdown
Member

It's fine to use AI assistance, just disclose it (e.g. via a Co-authored-by line in the commit message or so).

@fingolfin fingolfin added the release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes label Apr 20, 2026
Closes gap-system#6004.

The GAPDoc description for IrreducibleModules claimed the second entry
was 'a list of all irreducible modules of G over the field F in
dimension dim'. The actual behaviour (lib/grpreps.gi:45) is to return
all irreducible constituents of dimension at most dim, matching:

- The sibling AbsolutelyIrreducibleModules docstring in the same file
  (lib/grpreps.gd:18-23), which already says 'dimension at most dim'.
- The InstallOtherMethod at lib/grpreps.gi:314 titled 'Supply no
  dimension limit', which delegates to IrreducibleModules(G, F, 0) —
  only makes sense if 0 means 'no upper bound', not 'modules of
  dimension exactly 0'.

Updated the description to say 'of dimension at most <dim>', matching
the sibling operation, and added a one-line note that passing 0 imposes
no bound (already the implementation behavior, previously only
discoverable by reading the code).

Co-authored-by: Claude (Anthropic AI) <noreply@anthropic.com>
@mvanhorn mvanhorn force-pushed the osc/6004-irreducible-modules-doc branch from b6f4ad2 to b5ec59c Compare April 21, 2026 15:51
@mvanhorn
Copy link
Copy Markdown
Contributor Author

@fingolfin Done. Amended commit b5ec59c with Co-authored-by: Claude (Anthropic AI) to disclose AI assistance per your preference. Diff unchanged.

@fingolfin fingolfin enabled auto-merge (squash) April 21, 2026 21:57
@fingolfin fingolfin merged commit 131274e into gap-system:master Apr 23, 2026
33 checks passed
@mvanhorn
Copy link
Copy Markdown
Contributor Author

Thanks for landing the doc fix, @fingolfin.

cdwensley pushed a commit that referenced this pull request Apr 28, 2026
The GAPDoc description for IrreducibleModules claimed the second
entry was 'a list of all irreducible modules of G over the field F
in dimension dim'. The actual behaviour is to return all
irreducible constituents of dimension at most dim, matching:

Updated the description to match this, and also added a one-line
note that passing 0 imposes no bound (already the implementation
behavior, previously only discoverable by reading the code).

Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Co-authored-by: Claude (Anthropic AI) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes topic: documentation Issues and PRs related to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Documentation inaccuracy in IrreducibleModules

2 participants