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

Speed up polynomial mappings #4124

Merged
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
6fd471a
Attempt to speed up mapping of polynomials.
HechtiDerLachs Sep 19, 2024
70279f9
Some more steps towards making stuff faster.
HechtiDerLachs Sep 19, 2024
4304881
Try to make general evaluation faster.
HechtiDerLachs Sep 24, 2024
a96cf36
Clean up general evaluation.
HechtiDerLachs Sep 25, 2024
b6a3c40
Fix tests.
HechtiDerLachs Sep 30, 2024
d1f90e3
Widen type signature so that more rings benefit from variable-map.
HechtiDerLachs Sep 30, 2024
f98f198
Fix call.
HechtiDerLachs Sep 30, 2024
570e769
Implement extension for localizations and quotients.
HechtiDerLachs Sep 30, 2024
2d83dd6
Switch default for _allunique.
HechtiDerLachs Sep 30, 2024
9a59613
Fix tests.
HechtiDerLachs Sep 30, 2024
4e5b8e7
Remove superfluous if-clause and add some comments.
HechtiDerLachs Sep 30, 2024
f005ce1
Fix doctests in Algebraic statistics.
HechtiDerLachs Sep 30, 2024
0c65e56
Fix doctests in FTheoryTools.
HechtiDerLachs Sep 30, 2024
8944bb2
Fix doctests in AffineSchemes.
HechtiDerLachs Sep 30, 2024
90dc095
Revert "Fix doctests in FTheoryTools."
HechtiDerLachs Sep 30, 2024
5d61dda
Revert "Fix doctests in Algebraic statistics."
HechtiDerLachs Sep 30, 2024
7ad7f39
Remove duplicated code.
HechtiDerLachs Sep 30, 2024
4d17d35
Revert to a single internal constructor.
HechtiDerLachs Sep 30, 2024
f7289e8
Fix internal method.
HechtiDerLachs Sep 30, 2024
9092a8a
Remove some more code duplication.
HechtiDerLachs Sep 30, 2024
733b8f1
Switch to the AA methods.
HechtiDerLachs Oct 1, 2024
1892dab
Update src/Rings/MPolyMap/MPolyAnyMap.jl
simonbrandhorst Oct 1, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -506,3 +506,7 @@ function Base.show(io::IO, ::MIME"text/plain", a::AffineSchemeOpenSubschemeRingE
end
end

# overwrite a method for mapping of rings which would throw otherwise
function _allunique(lst::Vector{T}) where {T<:MPolyQuoRingElem{<:MPolyRingElem{<:AffineSchemeOpenSubschemeRingElem}}}
return false
end
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ Affine scheme morphism
from [x1, x2, x3] scheme(x1)
to [x1, x2, x3] affine 3-space over QQ
given by the pullback function
x1 -> 0
x1 -> x1
HechtiDerLachs marked this conversation as resolved.
Show resolved Hide resolved
x2 -> x2
x3 -> x3

Expand Down Expand Up @@ -95,7 +95,7 @@ Affine scheme morphism
from [x1, x2, x3] scheme(x1)
to [x1, x2, x3] affine 3-space over QQ
given by the pullback function
x1 -> 0
x1 -> x1
x2 -> x2
x3 -> x3

Expand Down Expand Up @@ -143,7 +143,7 @@ Ring homomorphism
from multivariate polynomial ring in 3 variables over QQ
to quotient of multivariate polynomial ring by ideal (x1)
defined by
x1 -> 0
x1 -> x1
x2 -> x2
x3 -> x3
```
Expand Down Expand Up @@ -254,7 +254,7 @@ Affine scheme morphism
from [x1, x2, x3] scheme(x1)
to [x1, x2, x3] affine 3-space over QQ
given by the pullback function
x1 -> 0
x1 -> x1
x2 -> x2
x3 -> x3

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ Affine scheme morphism
from [x1, x2, x3] scheme(x1)
to [x1, x2, x3] affine 3-space over QQ
given by the pullback function
x1 -> 0
x1 -> x1
x2 -> x2
x3 -> x3

Expand Down Expand Up @@ -181,7 +181,7 @@ Affine scheme morphism
from [x1, x2, x3] scheme(x1)
to [x1, x2, x3] affine 3-space over QQ
given by the pullback function
x1 -> 0
x1 -> x1
x2 -> x2
x3 -> x3

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ Affine scheme morphism
from [x, y] scheme(x)
to [x, y] affine 2-space over QQ
given by the pullback function
x -> 0
x -> x
y -> y

julia> inc == inclusion_morphism(Y, X)
Expand Down
31 changes: 23 additions & 8 deletions src/Rings/MPolyMap/MPolyAnyMap.jl
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,31 @@ const _DomainTypes = Union{MPolyRing, MPolyQuoRing}
coeff_map::U
img_gens::Vector{V}
temp_ring # temporary ring used when evaluating maps
variable_indices::Vector{Int} # a table where the i-th entry contains the
# index of the variable where it is mapped to
# in case the mapping takes such a particularly
# simple form.

function MPolyAnyMap{D, C, U, V}(domain::D,
codomain::C,
coeff_map::U,
img_gens::Vector{V}) where {D, C, U, V}
@assert V === elem_type(C)
for g in img_gens
@assert parent(g) === codomain "elements does not have the correct parent"
end
return new{D, C, U, V}(domain, codomain, coeff_map, img_gens)
codomain::C,
coeff_map::U,
img_gens::Vector{V};
check_for_mapping_of_vars::Bool=true
) where {D, C, U, V}
@assert V === elem_type(C)
for g in img_gens
@assert parent(g) === codomain "elements does not have the correct parent"
end
result = new{D, C, U, V}(domain, codomain, coeff_map, img_gens)
# If it ever turned out that doing the checks within the following if-block
# is a bottleneck, consider passing on the `check_for_mapping_of_vars` kw
# argument to the outer constructors or make the outer constructors
# call the inner one with this argument set to `false`. This way the check
# can safely be disabled.
if check_for_mapping_of_vars && all(_is_gen, img_gens) && _allunique(img_gens)
result.variable_indices = [findfirst(==(x), gens(codomain)) for x in img_gens]
simonbrandhorst marked this conversation as resolved.
Show resolved Hide resolved
simonbrandhorst marked this conversation as resolved.
Show resolved Hide resolved
end
return result
end
end

Expand Down
96 changes: 95 additions & 1 deletion src/Rings/MPolyMap/MPolyRing.jl
Original file line number Diff line number Diff line change
Expand Up @@ -132,17 +132,111 @@
#
################################################################################

# Some additional methods needed for the test in the constructor for MPolyAnyMap
_is_gen(x::MPolyQuoRingElem) = _is_gen(lift(x))
_is_gen(x::MPolyDecRingElem) = is_gen(forget_grading(x))
_is_gen(x::MPolyRingElem) = is_gen(x)
# default method; overwrite if you want this to work for your rings.
_is_gen(x::NCRingElem) = false

# In case there is a type of ring elements for which hashing is correctly implemented
# and does not throw an error, this gives the opportunity to overwrite the `allunique`
# to be used within the constructor for maps.
function _allunique(lst::Vector{T}) where {T<:MPolyRingElem}
return allunique(lst)
end

# We have a lot of rings which do/can not implement correct hashing.
# So we make the following the default.
function _allunique(lst::Vector{T}) where {T<:RingElem}
return all(!(x in lst[i+1:end]) for (i, x) in enumerate(lst))
end

function _build_poly(u::MPolyRingElem, indices::Vector{Int}, S::MPolyRing)
kk = coefficient_ring(S)
r = ngens(S)
ctx = MPolyBuildCtx(S)
for (c, e) in zip(AbstractAlgebra.coefficients(u), AbstractAlgebra.exponent_vectors(u))
ee = [0 for _ in 1:r]
for (i, k) in enumerate(e)
ee[indices[i]] = k
end
push_term!(ctx, kk(c), ee)
end
return finish(ctx)
end

function _evaluate_plain(F::MPolyAnyMap{<:MPolyRing, <:MPolyRing}, u)
isdefined(F, :variable_indices) && return _build_poly(u, F.variable_indices, codomain(F))
return evaluate(u, F.img_gens)
end

function _evaluate_plain(F::MPolyAnyMap{<: MPolyRing}, u)
return evaluate(u, F.img_gens)
end

# See the comment in MPolyQuo.jl
function _evaluate_plain(F::MPolyAnyMap{<:MPolyRing, <:MPolyQuoRing}, u)
A = codomain(F)
R = base_ring(A)
isdefined(F, :variable_indices) && return A(_build_poly(u, F.variable_indices, R))
v = evaluate(lift(u), lift.(_images(F)))
return simplify(A(v))
end

# The following assumes `p` to be in `S[x₁,…,xₙ]` where `S` is the
# actual codomain of the map.
function _evaluate_with_build_ctx(
p::MPolyRingElem, ind::Vector{Int}, cod_ring::MPolyRing
)
@assert cod_ring === coefficient_ring(parent(p))
r = ngens(cod_ring)
kk = coefficient_ring(cod_ring)
ctx = MPolyBuildCtx(cod_ring)
for (q, e) in zip(coefficients(p), exponents(p))
ee = [0 for _ in 1:r]
for (i, k) in enumerate(e)
ee[ind[i]] = k
end
for (c, d) in zip(coefficients(q), exponents(q))
push_term!(ctx, kk(c), ee+d)
end
end
return finish(ctx)
end

function _evaluate_general(F::MPolyAnyMap{<:MPolyRing, <:MPolyRing}, u)
if domain(F) === codomain(F) && coefficient_map(F) === nothing
return evaluate(map_coefficients(coefficient_map(F), u,

Check warning on line 210 in src/Rings/MPolyMap/MPolyRing.jl

View check run for this annotation

Codecov / codecov/patch

src/Rings/MPolyMap/MPolyRing.jl#L210

Added line #L210 was not covered by tests
parent = domain(F)), F.img_gens)
else
S = temp_ring(F)
if S !== nothing
if !isdefined(F, :variable_indices) || coefficient_ring(S) !== codomain(F)
return evaluate(map_coefficients(coefficient_map(F), u,
parent = S), F.img_gens)
else
tmp_poly = map_coefficients(coefficient_map(F), u, parent = S)
return _evaluate_with_build_ctx(tmp_poly, F.variable_indices, codomain(F))
end
else
if !isdefined(F, :variable_indices)
return evaluate(map_coefficients(coefficient_map(F), u), F.img_gens)
else
# For the case where we can recycle the method above, do so.
tmp_poly = map_coefficients(coefficient_map(F), u)
coefficient_ring(parent(tmp_poly)) === codomain(F) && return _evaluate_with_build_ctx(
tmp_poly,
F.variable_indices,
codomain(F)
)
# Otherwise default to the standard evaluation for the time being.
return evaluate(tmp_poly, F.img_gens)
end
end
end
end

function _evaluate_general(F::MPolyAnyMap{<: MPolyRing}, u)
if domain(F) === codomain(F) && coefficient_map(F) === nothing
return evaluate(map_coefficients(coefficient_map(F), u,
Expand All @@ -151,7 +245,7 @@
S = temp_ring(F)
if S !== nothing
return evaluate(map_coefficients(coefficient_map(F), u,
parent = S), F.img_gens)
parent = S), F.img_gens)
else
return evaluate(map_coefficients(coefficient_map(F), u), F.img_gens)
end
Expand Down
87 changes: 87 additions & 0 deletions src/Rings/mpolyquo-localizations.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2717,3 +2717,90 @@
return dim(modulus(R))
end

### extra methods for speedup of mappings
# See `src/Rings/MPolyMap/MPolyRing.jl` for the original implementation and
# the rationale. These methods make speedup available for quotient rings
# and localizations of polynomial rings.
function _allunique(lst::Vector{T}) where {T<:MPolyLocRingElem}
return _allunique(numerator.(lst))
end

function _allunique(lst::Vector{T}) where {T<:MPolyQuoLocRingElem}
return _allunique(lifted_numerator.(lst))
end

_is_gen(x::MPolyLocRingElem) = is_one(denominator(x)) && _is_gen(numerator(x))
_is_gen(x::MPolyQuoLocRingElem) = is_one(lifted_denominator(x)) && _is_gen(lifted_numerator(x))

function _evaluate_plain(
F::MPolyAnyMap{<:MPolyRing, CT}, u
) where {CT <: Union{<:MPolyLocRing, <:MPolyQuoLocRing}}
W = codomain(F)::MPolyLocRing
S = base_ring(W)
isdefined(F, :variable_indices) && return W(_build_poly(u, F.variable_indices, S))
return evaluate(u, F.img_gens)
end

function _evaluate_general(
F::MPolyAnyMap{<:MPolyRing, CT}, u
) where {CT <: Union{<:MPolyQuoRing, <:MPolyLocRing, <:MPolyQuoLocRing}}
S = temp_ring(F)
if S !== nothing
if !isdefined(F, :variable_indices) || coefficient_ring(S) !== codomain(F)
return evaluate(map_coefficients(coefficient_map(F), u,
parent = S), F.img_gens)
else
tmp_poly = map_coefficients(coefficient_map(F), u, parent = S)
return _evaluate_with_build_ctx(tmp_poly, F.variable_indices, codomain(F))
end
else
if !isdefined(F, :variable_indices)
return evaluate(map_coefficients(coefficient_map(F), u), F.img_gens)

Check warning on line 2758 in src/Rings/mpolyquo-localizations.jl

View check run for this annotation

Codecov / codecov/patch

src/Rings/mpolyquo-localizations.jl#L2758

Added line #L2758 was not covered by tests
else
# For the case where we can recycle the method above, do so.
tmp_poly = map_coefficients(coefficient_map(F), u)
coefficient_ring(parent(tmp_poly)) === codomain(F) && return _evaluate_with_build_ctx(
tmp_poly,
F.variable_indices,
codomain(F)
)
# Otherwise default to the standard evaluation for the time being.
return evaluate(tmp_poly, F.img_gens)
end
end
end

function _evaluate_with_build_ctx(
p::MPolyRingElem, ind::Vector{Int},
Q::Union{<:MPolyQuoRing, <:MPolyLocRing, <:MPolyQuoLocRing}
)
@assert Q === coefficient_ring(parent(p))
cod_ring = base_ring(Q)
r = ngens(cod_ring)
kk = coefficient_ring(cod_ring)
ctx = MPolyBuildCtx(cod_ring)
for (q, e) in zip(coefficients(p), exponents(p))
ee = [0 for _ in 1:r]
for (i, k) in enumerate(e)
ee[ind[i]] = k
end
for (c, d) in zip(_coefficients(q), _exponents(q))
push_term!(ctx, kk(c), ee+d)
end
end
return Q(finish(ctx))
end

# The following methods are only safe, because they are called
# exclusively in a setup where variables map to variables
# and no denominators are introduced.
_coefficients(x::MPolyRingElem) = coefficients(x)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure, but maybe you want the following here?

Suggested change
_coefficients(x::MPolyRingElem) = coefficients(x)
_coefficients(x::MPolyRingElem) = AbstractAlgebra.coefficients(x)

And also below with exponent_vectors.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I saw the AA-qualifier elsewhere, too. But I thought, it was redundant. Isn't it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No. It is the difference between painfully slow and fast when iterating over coefficients/exponents. There used to be an explanation in the documentation, but it was removed for reasons I don't know.

Copy link
Collaborator

Choose a reason for hiding this comment

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

See #4168

_coefficients(x::MPolyQuoRingElem) = coefficients(lift(x))
_coefficients(x::MPolyLocRingElem) = coefficients(numerator(x))
_coefficients(x::MPolyQuoLocRingElem) = coefficients(lifted_numerator(x))

_exponents(x::MPolyRingElem) = exponents(x)
_exponents(x::MPolyQuoRingElem) = exponents(lift(x))
_exponents(x::MPolyLocRingElem) = exponents(numerator(x))
_exponents(x::MPolyQuoLocRingElem) = exponents(lifted_numerator(x))

Loading