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

Some improvements on the modules #3041

Merged
merged 16 commits into from
Nov 30, 2023
Merged
Show file tree
Hide file tree
Changes from 9 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
95 changes: 64 additions & 31 deletions src/Modules/ModuleTypes.jl
Original file line number Diff line number Diff line change
Expand Up @@ -294,24 +294,29 @@
true
```
"""
struct SubquoModuleElem{T} <: AbstractSubQuoElem{T}
coeffs::SRow{T}
repres::FreeModElem{T}
parent::SubquoModule
mutable struct SubquoModuleElem{T} <: AbstractSubQuoElem{T}
parent::SubquoModule{T}
coeffs::SRow{T} # Need not be defined! Use `coordinates` as getter
repres::FreeModElem{T} # Need not be defined! Use `repres` as getter
is_reduced::Bool # `false` by default. Will be set by `simplify` and `simplify!`.

function SubquoModuleElem{R}(v::SRow{R}, SQ::SubquoModule) where {R}
@assert length(v) <= ngens(SQ.sub)
if isempty(v)
r = new{R}(v, zero(SQ.F), SQ)
r = new{R}(SQ, v, zero(SQ.F))
return r
end
r = new{R}(v, sum([v[i]*SQ.sub[i] for i=1:ngens(SQ.sub)]), SQ)
r = new{R}(SQ, v)
#, sum(a*SQ.sub[i] for (i, a) in v; init = zero(SQ.sub)), SQ)
r.is_reduced = false
return r
end

function SubquoModuleElem{R}(a::FreeModElem{R}, SQ::SubquoModule) where {R}
function SubquoModuleElem{R}(a::FreeModElem{R}, SQ::SubquoModule; is_reduced::Bool=false) where {R}
@assert a.parent === SQ.F
r = new{R}(coordinates(a,SQ), a, SQ)
r = new{R}(SQ)
r.repres = a
r.is_reduced = is_reduced
return r
end
end
Expand All @@ -324,13 +329,17 @@
} <: ModuleFPHom{T1, T2, RingMapType}
matrix::MatElem
header::Hecke.MapHeader
im::Vector
im::Vector # The images of the generators; use `images_of_generators` as a getter.
inverse_isomorphism::ModuleFPHom
ring_map::RingMapType
d::GrpAbFinGenElem
generators_map_to_generators::Union{Bool, Nothing} # A flag to allow for shortcut in evaluation;
# value `nothing` by default and to be set manually.

# Constructors for maps without change of base ring
function SubQuoHom{T1,T2,RingMapType}(D::SubquoModule, C::FreeMod, im::Vector) where {T1,T2,RingMapType}
function SubQuoHom{T1,T2,RingMapType}(D::SubquoModule, C::FreeMod, im::Vector;
check::Bool=true
) where {T1,T2,RingMapType}
###@assert is_graded(D) == is_graded(C)
@assert length(im) == ngens(D)
@assert all(x-> parent(x) === C, im)
Expand All @@ -339,11 +348,14 @@
r.header = Hecke.MapHeader(D, C)
r.header.image = x->image(r, x)
r.header.preimage = x->preimage(r, x)
r.im = Vector{FreeModElem}(im)
r.im = Vector{elem_type(C)}(im)
r.generators_map_to_generators = nothing
return set_grading(r)
end

function SubQuoHom{T1,T2,RingMapType}(D::SubquoModule, C::SubquoModule, im::Vector) where {T1,T2,RingMapType}
function SubQuoHom{T1,T2,RingMapType}(D::SubquoModule, C::SubquoModule, im::Vector;
check::Bool=true
) where {T1,T2,RingMapType}
###@assert is_graded(D) == is_graded(C)
@assert length(im) == ngens(D)
@assert all(x-> parent(x) === C, im)
Expand All @@ -352,11 +364,14 @@
r.header = Hecke.MapHeader(D, C)
r.header.image = x->image(r, x)
r.header.preimage = x->preimage(r, x)
r.im = Vector{SubquoModuleElem}(im)
r.im = Vector{elem_type(C)}(im)
r.generators_map_to_generators = nothing
return set_grading(r)
end

function SubQuoHom{T1,T2,RingMapType}(D::SubquoModule, C::ModuleFP, im::Vector) where {T1,T2,RingMapType}
function SubQuoHom{T1,T2,RingMapType}(D::SubquoModule, C::ModuleFP, im::Vector;

Check warning on line 372 in src/Modules/ModuleTypes.jl

View check run for this annotation

Codecov / codecov/patch

src/Modules/ModuleTypes.jl#L372

Added line #L372 was not covered by tests
check::Bool=true
) where {T1,T2,RingMapType}
###@assert is_graded(D) == is_graded(C)
@assert length(im) == ngens(D)
@assert all(x-> parent(x) === C, im)
Expand All @@ -365,7 +380,8 @@
r.header = Hecke.MapHeader(D, C)
r.header.image = x->image(r, x)
r.header.preimage = x->preimage(r, x)
r.im = im
r.im = Vector{elem_type(C)}(im)
r.generators_map_to_generators = nothing

Check warning on line 384 in src/Modules/ModuleTypes.jl

View check run for this annotation

Codecov / codecov/patch

src/Modules/ModuleTypes.jl#L383-L384

Added lines #L383 - L384 were not covered by tests
return set_grading(r)
end

Expand All @@ -374,7 +390,8 @@
D::SubquoModule,
C::FreeMod,
im::Vector,
h::RingMapType
h::RingMapType;
check::Bool=true
) where {T1,T2,RingMapType}
###@assert is_graded(D) == is_graded(C)
@assert length(im) == ngens(D)
Expand All @@ -384,16 +401,18 @@
r.header = Hecke.MapHeader(D, C)
r.header.image = x->image(r, x)
r.header.preimage = x->preimage(r, x)
r.im = Vector{FreeModElem}(im)
r.im = Vector{elem_type(C)}(im)
r.ring_map = h
r.generators_map_to_generators = nothing
return set_grading(r)
end

function SubQuoHom{T1,T2,RingMapType}(
D::SubquoModule,
C::SubquoModule,
im::Vector,
h::RingMapType
h::RingMapType;
check::Bool=true
) where {T1,T2,RingMapType}
###@assert is_graded(D) == is_graded(C)
@assert length(im) == ngens(D)
Expand All @@ -403,16 +422,18 @@
r.header = Hecke.MapHeader(D, C)
r.header.image = x->image(r, x)
r.header.preimage = x->preimage(r, x)
r.im = Vector{SubquoModuleElem}(im)
r.im = Vector{elem_type(C)}(im)
r.ring_map = h
r.generators_map_to_generators = nothing
return set_grading(r)
end

function SubQuoHom{T1,T2,RingMapType}(
D::SubquoModule,
C::ModuleFP,
im::Vector,
h::RingMapType
h::RingMapType;
check::Bool=true
) where {T1,T2,RingMapType}
###@assert is_graded(D) == is_graded(C)
@assert length(im) == ngens(D)
Expand All @@ -422,8 +443,9 @@
r.header = Hecke.MapHeader(D, C)
r.header.image = x->image(r, x)
r.header.preimage = x->preimage(r, x)
r.im = im
r.im = Vector{elem_type(C)}(im)

Check warning on line 446 in src/Modules/ModuleTypes.jl

View check run for this annotation

Codecov / codecov/patch

src/Modules/ModuleTypes.jl#L446

Added line #L446 was not covered by tests
r.ring_map = h
r.generators_map_to_generators = nothing

Check warning on line 448 in src/Modules/ModuleTypes.jl

View check run for this annotation

Codecov / codecov/patch

src/Modules/ModuleTypes.jl#L448

Added line #L448 was not covered by tests
return set_grading(r)
end

Expand Down Expand Up @@ -506,9 +528,13 @@
header::MapHeader
ring_map::RingMapType
d::GrpAbFinGenElem
imgs_of_gens::Vector # stored here for easy evaluation; use `images_of_generators` as getter

matrix::MatElem
inverse_isomorphism::ModuleFPHom
generators_map_to_generators::Union{Bool, Nothing} # A flag to allow for shortcut in evaluation
# of the map; `nothing` by default and to be
# set manually.

# generate homomorphism of free modules from F to G where the vector a contains the images of
# the generators of F
Expand All @@ -519,19 +545,22 @@
@assert all(x->parent(x) === G, a)
@assert length(a) == ngens(F)
r = new{typeof(F), typeof(G), Nothing}()
a=Vector{elem_type(G)}(a)
function im_func(x::AbstractFreeModElem)
b = zero(G)
for (i,v) = x.coords
b += v*a[i]
end
return b
#if r.generators_map_to_generators === nothing
# r.generators_map_to_generators = images_of_generators(r) == gens(codomain(r))
#end
r.generators_map_to_generators === true && return codomain(r)(coordinates(x))
return sum(b*a[i] for (i, b) in coordinates(x); init=zero(codomain(r)))
end
function pr_func(x)
@assert parent(x) === G
c = coordinates(repres(x), sub(G, a, :module))
return FreeModElem(c, F)
end
r.header = MapHeader{typeof(F), typeof(G)}(F, G, im_func, pr_func)
r.imgs_of_gens = Vector{elem_type(G)}(a)
r.generators_map_to_generators = nothing
return set_grading(r)
end

Expand All @@ -543,12 +572,14 @@
@assert length(a) == ngens(F)
@assert h(one(base_ring(F))) == one(base_ring(G))
r = new{typeof(F), T2, RingMapType}()
a=Vector{elem_type(G)}(a)
function im_func(x::AbstractFreeModElem)
b = zero(G)
for (i,v) = x.coords
b += h(v)*a[i]
end
return b
iszero(x) && return zero(codomain(r))
#if r.generators_map_to_generators === nothing
# r.generators_map_to_generators = images_of_generators(r) == gens(codomain(r))
#end
r.generators_map_to_generators === true && return codomain(r)(map_entries(h, coordinates(x)))
return sum(h(b)*a[i] for (i, b) in coordinates(x); init=zero(codomain(r)))
end
function pr_func(x)
@assert parent(x) === G
Expand All @@ -558,6 +589,8 @@
end
r.header = MapHeader{typeof(F), T2}(F, G, im_func, pr_func)
r.ring_map = h
r.imgs_of_gens = Vector{elem_type(G)}(a)
r.generators_map_to_generators = nothing
return set_grading(r)
end

Expand Down
18 changes: 9 additions & 9 deletions src/Modules/ModulesGraded.jl
Original file line number Diff line number Diff line change
Expand Up @@ -976,13 +976,13 @@
```
"""
function is_homogeneous(el::SubquoModuleElem)
if iszero(el.coeffs)
if iszero(coordinates(el))
return is_homogeneous(repres(el))
else
degree = determine_degree_from_SR(el.coeffs, degrees_of_generators(parent(el)))
degree = determine_degree_from_SR(coordinates(el), degrees_of_generators(parent(el)))
if degree === nothing
reduced_el = simplify(el)
degree_reduced = determine_degree_from_SR(reduced_el.coeffs, degrees_of_generators(parent(reduced_el)))
degree_reduced = determine_degree_from_SR(coordinates(reduced_el), degrees_of_generators(parent(reduced_el)))
return degree_reduced !== nothing
else
return true
Expand Down Expand Up @@ -1040,11 +1040,11 @@
```
"""
function degree(el::SubquoModuleElem)
if !iszero(el.coeffs)
result = determine_degree_from_SR(el.coeffs, degrees_of_generators(parent(el)))
if !iszero(coordinates(el))
result = determine_degree_from_SR(coordinates(el), degrees_of_generators(parent(el)))
if result === nothing
reduced_el = simplify(el)
result_reduced = determine_degree_from_SR(reduced_el.coeffs, degrees_of_generators(parent(reduced_el)))
result_reduced = determine_degree_from_SR(coordinates(reduced_el), degrees_of_generators(parent(reduced_el)))
@assert result_reduced !== nothing "The specified element is not homogeneous."
return result_reduced
else
Expand Down Expand Up @@ -1422,11 +1422,11 @@
end
else
parent(b.project) == parent(x[1][2]) || error("projection vector has wrong type")
print(io, "Betti Table for scalar product of grading with ", b.project.coeff, "\n")
print(io, "Betti Table for scalar product of grading with ", coordinates(b.project), "\n")

Check warning on line 1425 in src/Modules/ModulesGraded.jl

View check run for this annotation

Codecov / codecov/patch

src/Modules/ModulesGraded.jl#L1425

Added line #L1425 was not covered by tests
print(io, " ")
L = Vector{ZZRingElem}(undef,0)
for i in 1:length(x)
temp_sum = (b.project.coeff * transpose(x[i][2].coeff))[1]
temp_sum = (coordinates(b.project) * transpose(coordinates(x[i][2])))[1]

Check warning on line 1429 in src/Modules/ModulesGraded.jl

View check run for this annotation

Codecov / codecov/patch

src/Modules/ModulesGraded.jl#L1429

Added line #L1429 was not covered by tests
Base.push!(L, temp_sum)
end
L1 = sort(unique(L))
Expand All @@ -1442,7 +1442,7 @@
for h in min:step:max
partial_sum = 0
for i in 1:length(x)
current_sum = (b.project.coeff * transpose(x[i][2].coeff))[1]
current_sum = (coordinates(b.project) * transpose(coordinates(x[i][2])))[1]

Check warning on line 1445 in src/Modules/ModulesGraded.jl

View check run for this annotation

Codecov / codecov/patch

src/Modules/ModulesGraded.jl#L1445

Added line #L1445 was not covered by tests
if current_sum == L1[k] && x[i][1] == h
partial_sum += getindex(T, x[i])
end
Expand Down
4 changes: 2 additions & 2 deletions src/Modules/UngradedModules/DirectSum.jl
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ Additionally, return
"""
function direct_product(M::ModuleFP{T}...; task::Symbol = :prod) where T
F, pro, mF = direct_product([ambient_free_module(x) for x = M]..., task = :both)
s, emb_sF = sub(F, vcat([[mF[i](y) for y = ambient_representatives_generators(M[i])] for i=1:length(M)]...), :both)
q::Vector{elem_type(F)} = vcat([[mF[i](y) for y = rels(M[i])] for i=1:length(M)]...)
s, emb_sF = sub(F, vcat([elem_type(F)[mF[i](y) for y = ambient_representatives_generators(M[i])] for i=1:length(M)]...), :both)
q::Vector{elem_type(F)} = vcat([elem_type(F)[mF[i](y) for y = rels(M[i])] for i=1:length(M)]...)
pro_quo = nothing
if length(q) != 0
s, pro_quo = quo(s, q, :both)
Expand Down
14 changes: 13 additions & 1 deletion src/Modules/UngradedModules/FreeModElem.jl
Original file line number Diff line number Diff line change
Expand Up @@ -216,14 +216,26 @@ function (==)(a::AbstractFreeModElem, b::AbstractFreeModElem)
return a.coords == b.coords
end

function hash(a::AbstractFreeModElem, h::UInt)
function hash(a::AbstractFreeModElem{<:MPolyElem}, h::UInt)
b = 0xaa2ba4a32dd0b431 % UInt
h = hash(typeof(a), h)
h = hash(parent(a), h)
h = hash(coordinates(a), h)
return xor(h, b)
end

function hash(a::AbstractFreeModElem{<:MPolyQuoRingElem}, h::UInt)
simplify!(a)
Copy link
Member

Choose a reason for hiding this comment

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

We just discussed how this call resp. this hash method is somewhat dangerous:

  • it assumes that simplify!(a) modifies a in-place and also "remembers" that it is simplified (so it is fast "amortized", i.e. you only compute a GB for it once)
  • it must produce a unique normal form -- but we don't really have a specification for simplify!. We really need one otherwise we are building on quicksand
  • perhaps it would be better to just do error("not supported") here and handle using these in dicts differently??
  • however the old hash(a::AbstractFreeModElem, h::UInt) method just was wrong...

b = 0xaa2ba4a32dd0b431 % UInt
h = hash(typeof(a), h)
h = hash(parent(a), h)
h = hash(coordinates(a), h)
return xor(h, b)
end

simplify(a::AbstractFreeModElem{<:MPolyQuoRingElem}) = return parent(a)(map_entries(simplify, coordinates(a)))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
simplify(a::AbstractFreeModElem{<:MPolyQuoRingElem}) = return parent(a)(map_entries(simplify, coordinates(a)))
simplify(a::AbstractFreeModElem{<:MPolyQuoRingElem}) = parent(a)(map_entries(simplify, coordinates(a)))

simplify!(a::AbstractFreeModElem{<:MPolyQuoRingElem}) = simplify(a)

function Base.deepcopy_internal(a::AbstractFreeModElem, dict::IdDict)
return parent(a)(deepcopy_internal(coordinates(a), dict))
end
Expand Down
Loading
Loading