Skip to content

Document the relation between "abelian invariants" and "elementary divisors"#4888

Merged
fingolfin merged 2 commits intooscar-system:masterfrom
ThomasBreuer:TB_el_div_ab_inv
May 26, 2025
Merged

Document the relation between "abelian invariants" and "elementary divisors"#4888
fingolfin merged 2 commits intooscar-system:masterfrom
ThomasBreuer:TB_el_div_ab_inv

Conversation

@ThomasBreuer
Copy link
Copy Markdown
Member

  • rename the conversion functions between the Vector formats returned by abelian_invariants and elementary_divisors for groups: call them abelian_invariants and elementary_divisors

  • document these conversion functions

  • make their signatures more restrictive: request a Vector{T} where T <: IntegerUnion instead of just a Vector. (Thus one can no longer enter [] as an argument, for the invariants of the trivial group, Int[] or so is needed.)

This addresses issue #2968.

Alternatively, we could move the definition of abelian_invariants (for FinGenAbGroup) to Hecke.jl, add cross-references between abelian_invariants and elementary_divisors there, and then extend the two functions to GAPGroup in Oscar.jl.

One ugly detail in the current pull request is that referencing abelian_invariants(::Vector) from the docstring of abelian_invariants(G::Union{GAPGroup, FinGenAbGroup}) apparently needs the full signature abelian_invariants(::Vector{S}) where S <: Oscar.IntegerUnion, otherwise I get a wrong circular cross-reference to the docstring itself.
(Referencing elementary_divisors(::Vector) is not a problem, perhaps because elementary_divisors for groups is defined in Hecke.jl.)

- rename the conversion functions between the `Vector` formats
  returned by `abelian_invariants` and `elementary_divisors` for groups:
  call them `abelian_invariants` and `elementary_divisors`

- document these conversion functions

- make their signatures more restrictive:
  request a `Vector{T} where T <: IntegerUnion` instead of just a `Vector`.
  (Thus one can no longer enter `[]` as an argument, for the invariants of
  the trivial group, `Int[]` or so is needed.)
@codecov
Copy link
Copy Markdown

codecov Bot commented May 15, 2025

Codecov Report

Attention: Patch coverage is 96.42857% with 1 line in your changes missing coverage. Please review.

Project coverage is 84.92%. Comparing base (60ffc34) to head (c124fbc).
Report is 46 commits behind head on master.

Files with missing lines Patch % Lines
src/Groups/GrpAb.jl 96.29% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4888      +/-   ##
==========================================
+ Coverage   84.88%   84.92%   +0.03%     
==========================================
  Files         681      685       +4     
  Lines       91621    92183     +562     
==========================================
+ Hits        77772    78282     +510     
- Misses      13849    13901      +52     
Files with missing lines Coverage Δ
src/Groups/sub.jl 95.72% <100.00%> (ø)
src/Groups/GrpAb.jl 93.43% <96.29%> (+0.31%) ⬆️

... and 72 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread src/Groups/GrpAb.jl Outdated
Comment thread src/Groups/GrpAb.jl Outdated
@ThomasBreuer ThomasBreuer requested a review from fingolfin May 21, 2025 13:20
@fingolfin fingolfin merged commit d37b600 into oscar-system:master May 26, 2025
33 of 35 checks passed
@ThomasBreuer ThomasBreuer deleted the TB_el_div_ab_inv branch May 26, 2025 08:41
@lgoettgens
Copy link
Copy Markdown
Member

@ThomasBreuer @fingolfin could you please add appropriate release notes labels to this PR?

@ThomasBreuer ThomasBreuer changed the title relation between "abelian invariants" and "elementary divisors" document the relation between "abelian invariants" and "elementary divisors" May 27, 2025
@ThomasBreuer ThomasBreuer added the release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes label May 27, 2025
@ThomasBreuer ThomasBreuer changed the title document the relation between "abelian invariants" and "elementary divisors" Document the relation between "abelian invariants" and "elementary divisors" May 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes topic: groups

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants