Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

extend the default BrauerTableOp method #3129

Merged

Conversation

ThomasBreuer
Copy link
Contributor

  • extended the default BrauerTableOp method to the case
    of cyclic defect such that all Brauer characters lift
    to characteristic zero (this case can be detected from the
    ordinary character table)

  • extended the documentation accordingly

  • extended the tests accordingly

@ThomasBreuer ThomasBreuer added kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements topic: library release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Dec 20, 2018
Copy link
Member

@olexandr-konovalov olexandr-konovalov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me (failed tests caused by the problem with Julia that should be fixed by #3131). You may wish to rebase it to have a clean pass, and look at two suggestions.

I was also wondering if GAP should have a PPart function. It has PValuation, but will it be better to use p^PValuation(n,p) instead of the loop that you use to calculate the ppart - I am not sure.

lib/ctbl.gd Outdated Show resolved Hide resolved
n:= n / p;
od;
if ppart in OrdersClassRepresentatives( tbl ) then
# The Sylow 'p'-subgroup is cyclic.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to remove quotes around p to ease reading.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ThomasBreuer just to explain why: I read that first as p'-subgroup, then reread to figure out what does that mean. The style you use here is not consistent with other styles used in the GAP library: for example we have

grp.gi:      # compute generators for the torsion Omega p-subgroups of the center
grpprmcs.gi:##  O_p(G), the p-core of G, is the maximal normal p-subgroup
grpprmcs.gi:                        # abelian normal p-subgroup of G
grpprmcs.gi:    # p-subgroup of G; taking commutator subgroups, find abelian normal
grpprmcs.gi:    # p-subgroup of G.
grpprmcs.gi:    # p-subgroup; kernel is abelian p-group. Take image at this action, and

and also one case of

ctblsolv.gi:    # Choose a normal elementary abelian `p'-subgroup `N',

- extended the default `BrauerTableOp` method to the case
  of cyclic defect such that all Brauer characters lift
  to characteristic zero (this case can be detected from the
  ordinary character table)

- extended the documentation accordingly

- extended the tests accordingly
@codecov
Copy link

codecov bot commented Dec 21, 2018

Codecov Report

Merging #3129 into master will increase coverage by <.01%.
The diff coverage is 92.1%.

@@            Coverage Diff             @@
##           master    #3129      +/-   ##
==========================================
+ Coverage   83.58%   83.59%   +<.01%     
==========================================
  Files         687      687              
  Lines      336919   336957      +38     
==========================================
+ Hits       281619   281671      +52     
+ Misses      55300    55286      -14
Impacted Files Coverage Δ
lib/ctbl.gd 97.68% <ø> (ø) ⬆️
lib/ctbl.gi 79.01% <92.1%> (+0.54%) ⬆️
src/iostream.c 61.59% <0%> (-0.77%) ⬇️
src/stringobj.c 95.03% <0%> (+0.15%) ⬆️
hpcgap/lib/hpc/stdtasks.g 72.18% <0%> (+0.4%) ⬆️

@ThomasBreuer ThomasBreuer merged commit 344ae6f into gap-system:master Dec 21, 2018
@ThomasBreuer ThomasBreuer deleted the TB_liftable_Brauer_characters branch December 21, 2018 14:13
@DominikBernhardt DominikBernhardt added release notes: added PRs introducing changes that have since been mentioned in the release notes and removed release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Aug 22, 2019
@olexandr-konovalov olexandr-konovalov added this to the GAP 4.11.0 milestone Feb 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: added PRs introducing changes that have since been mentioned in the release notes topic: library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants