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

Rename rank(A::GrpAbFinGen) to torsion_free_rank #1224

Merged
merged 1 commit into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/src/abelian/introduction.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ nrels(G::FinGenAbGroup)
rels(A::FinGenAbGroup)
is_finite(A::FinGenAbGroup)
is_infinite(A::FinGenAbGroup)
rank(A::FinGenAbGroup)
torsion_free_rank(A::FinGenAbGroup)
order(A::FinGenAbGroup)
exponent(A::FinGenAbGroup)
is_trivial(A::FinGenAbGroup)
Expand Down
38 changes: 28 additions & 10 deletions src/GrpAb/GrpAbFinGen.jl
Original file line number Diff line number Diff line change
Expand Up @@ -533,20 +533,27 @@
################################################################################

@doc raw"""
rank(A::FinGenAbGroup) -> Int
torsion_free_rank(A::FinGenAbGroup) -> Int

Return the rank of $A$, that is, the dimension of the
Return the torsion free rank of $A$, that is, the dimension of the
$\mathbf{Q}$-vectorspace $A \otimes_{\mathbf Z} \mathbf Q$.
"""
rank(A::FinGenAbGroup) = is_snf(A) ? rank_snf(A) : rank_gen(A)

rank_snf(A::FinGenAbGroup) = length(findall(x -> iszero(x), A.snf))

rank_gen(A::FinGenAbGroup) = rank(snf(A)[1])

rank(A::FinGenAbGroup, p::Union{Int, ZZRingElem}) = is_snf(A) ? rank_snf(A, p) : rank_snf(snf(A)[1], p)
See also [`rank`](@ref).
"""
function torsion_free_rank(A::FinGenAbGroup)
if !is_snf(A)
A = snf(A)[1]
end
return length(findall(iszero, A.snf))
end

function rank_snf(A::FinGenAbGroup, p::Union{Int, ZZRingElem})
# return the p-rank of A, which is the dimension of the elementary abelian
# group A/pA, or in other words, the rank (=size of minimal generating set) of
# the maximal p-quotient of A
function rank(A::FinGenAbGroup, p::Union{Int, ZZRingElem})
if !is_snf(A)
A = snf(A)[1]

Check warning on line 555 in src/GrpAb/GrpAbFinGen.jl

View check run for this annotation

Codecov / codecov/patch

src/GrpAb/GrpAbFinGen.jl#L553-L555

Added lines #L553 - L555 were not covered by tests
end
if isempty(A.snf)
return 0
end
Expand All @@ -558,6 +565,17 @@
end


@doc raw"""
rank(A::FinGenAbGroup) -> Int

Return the rank of $A$, that is, the size of a minimal generating set for $A$.

See also [`torsion_free_rank`](@ref).
"""
rank(A::FinGenAbGroup) = error("rank(::FinGenAbGroup) has been renamed to torsion_free_rank")

Check warning on line 575 in src/GrpAb/GrpAbFinGen.jl

View check run for this annotation

Codecov / codecov/patch

src/GrpAb/GrpAbFinGen.jl#L575

Added line #L575 was not covered by tests
#rank(A::FinGenAbGroup) = is_snf(A) ? length(A.snf) : return rank(snf(A)[1])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am happy to activate the second method. The one concern I have is that existing code may silently break if we change the meaning of rank immediately. With this error I was hoping to catch at least errors in the Hecke and OscarCI test suites cased by this. We can of course change it before merging.



################################################################################
#
# Order
Expand Down
1 change: 1 addition & 0 deletions src/exports.jl
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,7 @@ export tensor_product
export theta
export tidy_model
export torsion_bound
export torsion_free_rank
export torsion_points
export torsion_points_division_poly
export torsion_points_lutz_nagell
Expand Down
12 changes: 8 additions & 4 deletions test/GrpAb/GrpAbFinGen.jl
Original file line number Diff line number Diff line change
Expand Up @@ -130,16 +130,20 @@

@testset "Rank" begin
G = abelian_group([3, 15])
@test @inferred rank(G) == 0
@test @inferred torsion_free_rank(G) == 0
#@test @inferred rank(G) == 2

G = abelian_group([3, 5])
@test @inferred rank(G) == 0
@test @inferred torsion_free_rank(G) == 0
#@test @inferred rank(G) == 1

G = abelian_group([3, 15, 0])
@test @inferred rank(G) == 1
@test @inferred torsion_free_rank(G) == 1
#@test @inferred rank(G) == 3

G = abelian_group([3, 5, 0])
@test @inferred rank(G) == 1
@test @inferred torsion_free_rank(G) == 1
#@test @inferred rank(G) == 2
end

@testset "Order" begin
Expand Down
Loading