Skip to content
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
21 changes: 1 addition & 20 deletions src/Rings/MPolyMap/AffineAlgebras.jl
Original file line number Diff line number Diff line change
Expand Up @@ -229,32 +229,13 @@ function preimage(
return _preimage_via_singular(f, I)
end

@doc raw"""
_maps_variables_to_variables(f::MPolyAnyMap)

A cheap check to see whether `f` is taking the variables of its domain into
pairwise different variables of its codomain. In the affirmative case
return `(true, ind)` where `ind` is a `Vector` of the indices so that
`gens(codomain(f))[ind]` equals the images of the generators.
Otherwise, return `(false, Int[])`.

Note: For rings different from plain polynomial rings in the codomain
this is not a rigorous check (which would probably be very expensive),
but based only a quick look on the representatives!
"""
function _maps_variables_to_variables(f::MPolyAnyMap)
isdefined(f, :variable_indices) && return true, copy(f.variable_indices)
(all(_is_gen, _images(f)) && _allunique(_images(f))) || return false, Int[]
f.variable_indices = [findfirst(_cmp_reps(x), gens(codomain(f))) for x in _images(f)]
return true, copy(f.variable_indices)
end

function preimage(
f::MPolyAnyMap{S, S, Nothing},
I::MPolyIdeal
) where {S <: MPolyRing{<:RingElem}}
# If the map is the inclusion of a subring in a subset of
# variables, use the `eliminate` command instead.
_assert_has_maps_variables_to_variables!(f)
success, ind = _maps_variables_to_variables(f)
if success
img_gens = elem_type(domain(f))[] # for the projection map
Expand Down
62 changes: 62 additions & 0 deletions src/Rings/MPolyMap/MPolyAnyMap.jl
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,73 @@ coefficient_map(f::MPolyAnyMap) = f.coeff_map

_images(f::MPolyAnyMap) = f.img_gens

@doc raw"""
_maps_variables_to_variables(f::MPolyAnyMap)

A cheap check to see whether `f` is taking the variables of its domain into
pairwise different variables of its codomain. In the affirmative case
return `(true, ind)` where `ind` is a `Vector` of the indices so that
`gens(codomain(f))[ind]` equals the images of the generators.
Otherwise, return `(false, garbage)`.

Note: For rings different from plain polynomial rings in the codomain
this is not a rigorous check (which would probably be very expensive),
but based only a quick look on the representatives!

Do not mutate the second return value.
"""
function _maps_variables_to_variables(f::MPolyAnyMap)
if ngens(domain(f)) == 0
return true, f.variable_indices
end
return !isempty(f.variable_indices), f.variable_indices
end

# This can be overwritten in order to avoid making the above check a bottleneck.
_cmp_reps(a) = ==(a)

function _assert_has_maps_variables_to_variables!(f::MPolyAnyMap)
if !isdefined(f, :variable_indices)
f.variable_indices = __maps_variables_to_variables(_images(f), codomain(f))
end
end

function __maps_variables_to_variables(img_gens::Vector, C)
# C is codomain

# this is a dirty implicit check, that the codomain is of a useful type for
# everything to make sense
# proper check would be to see if C isa MPolyRing etc
if !all(_is_gen, img_gens)
return Int[]
end

if !_allunique(img_gens)
return Int[]
end

l = length(img_gens)
r = Vector{Int}(undef, l)
Cgens = gens(C)

for i in 1:length(img_gens)
j = findfirst(_cmp_reps(img_gens[i]), Cgens)
if j isa Nothing
# image not a variable
return Int[]
end
r[i] = j::Int
end

return r
end

################################################################################
#
# String I/O
#
################################################################################
#
function Base.show(io::IO, ::MIME"text/plain", f::MPolyAnyMap)
io = pretty(io)
println(terse(io), f)
Expand Down
19 changes: 13 additions & 6 deletions src/Rings/MPolyMap/MPolyRing.jl
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,10 @@ function _build_poly(u::MPolyRingElem, indices::Vector{Int}, S::MPolyRing)
end

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

Expand All @@ -184,7 +187,10 @@ end
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))
fl, var_ind = _maps_variables_to_variables(F)
if fl
return A(_build_poly(u, var_ind, R))
end
v = evaluate(lift(u), lift.(_images(F)))
return simplify(A(v))
end
Expand Down Expand Up @@ -216,23 +222,24 @@ function _evaluate_general(F::MPolyAnyMap{<:MPolyRing, <:MPolyRing}, u)
parent = domain(F)), F.img_gens)
else
S = temp_ring(F)
fl, var_ind = _maps_variables_to_variables(F)
if S !== nothing
if !isdefined(F, :variable_indices) || coefficient_ring(S) !== codomain(F)
if !fl || 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))
return _evaluate_with_build_ctx(tmp_poly, var_ind, codomain(F))
end
else
if !isdefined(F, :variable_indices)
if !fl
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,
var_ind,
codomain(F)
)
# Otherwise default to the standard evaluation for the time being.
Expand Down
9 changes: 3 additions & 6 deletions src/Rings/MPolyMap/Types.jl
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,14 @@ const _DomainTypes = Union{MPolyRing, MPolyQuoRing}
# 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)
gens_codomain = gens(codomain)
result.variable_indices = [findfirst(_cmp_reps(x), gens_codomain) for x in img_gens]
if check_for_mapping_of_vars
result.variable_indices = __maps_variables_to_variables(img_gens,
codomain)
Comment on lines +36 to +38
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

In the previous implementation this field being unassigned was the indicator that the map does not take variables into variables. Now it seems that the field is assigned by default. But I suppose you deliberately changed the overall concept here and took that into account, right?

Either way: I was just a bit puzzled that this field is assigned only if a specific kwarg is set to true.

Copy link
Copy Markdown
Collaborator Author

@thofma thofma Mar 2, 2025

Choose a reason for hiding this comment

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

  1. Yes, I changed the concept so that there is now only one way to ask whether variables map to variables. Before there were two: Either call _maps_variables_to_variables or check isdefined(f, ...).
  2. The part with always setting the field is on purpose. Repeatedly constructing Int[] is expensive. Now one should never query isdefined(f, :) directly, but just ask whether _maps_... is true.
  3. The special kwarg was added by you. I am happy to remove it, since it is never used and cannot be used legally using hom, see Speed up polynomial mappings #4124 (comment).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I see. I think at the time I wanted to leave some backdoor open to avoid the check. That's why I introduced the kwarg, but did not yet forward it to the user-facing constructors. If it does not conflict with your implementation, then I would say: let's just keep it.

Not relying on isdefined anymore makes sense. This was a bit opaque, anyway.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

OK, let's keep keeping it. Thanks for the review.

end
return result
end
end

# This can be overwritten in order to avoid making the above check a bottleneck.
_cmp_reps(a) = ==(a)

function MPolyAnyMap(d::D, c::C, cm::U, ig::Vector{V}) where {D, C, U, V}
return MPolyAnyMap{D, C, U, V}(d, c, cm, ig)
end
Expand Down
27 changes: 18 additions & 9 deletions src/Rings/mpoly-ideals.jl
Original file line number Diff line number Diff line change
Expand Up @@ -443,32 +443,29 @@ Given an ordering `o` and an integer `l` return the `l`th elimination ideal for
This method will not compute a Groebner basis if one has already been computed for `o`.

!!! note
The return value is an ideal of the original ring.
The return value is an ideal of the original ring. To obtain the result in
a proper subring, construct an appropriate morphism and compute the
preimage of the ideal.

# Examples
```jldoctest
julia> R, (t, x, y, z) = polynomial_ring(QQ, [:t, :x, :y, :z])
(Multivariate polynomial ring in 4 variables over QQ, QQMPolyRingElem[t, x, y, z])
julia> R, (t, x, y, z) = polynomial_ring(QQ, [:t, :x, :y, :z]);

julia> I = ideal(R, [t-x, t^2-y, t^3-z])
Ideal generated by
t - x
t^2 - y
t^3 - z

julia> A = [t]
1-element Vector{QQMPolyRingElem}:
t
julia> A = [t];

julia> TC = eliminate(I, A)
Ideal generated by
-x*z + y^2
x*y - z
x^2 - y

julia> A = [1]
1-element Vector{Int64}:
1
julia> A = [1];

julia> TC = eliminate(I, A)
Ideal generated by
Expand All @@ -483,6 +480,18 @@ Ideal generated by
julia> base_ring(TC)
Multivariate polynomial ring in 4 variables t, x, y, z
over rational field

julia> S, (yy, zz) = QQ[:yy, :zz]; # Construct the elimination as an ideal of S

julia> f = hom(S, R, [y, z]);

julia> TCS = preimage(f, I)
Ideal generated by
yy^3 - zz^2

julia> base_ring(TCS)
Multivariate polynomial ring in 2 variables yy, zz
over rational field
```
"""
function eliminate(I::MPolyIdeal{T}, l::Vector{T}) where T <: MPolyRingElem
Expand Down
12 changes: 7 additions & 5 deletions src/Rings/mpolyquo-localizations.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2825,31 +2825,33 @@ function _evaluate_plain(
) 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))
fl, var_ind = _maps_variables_to_variables(F)
fl && return W(_build_poly(u, var_ind, 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)
fl, var_ind = _maps_variables_to_variables(F)
if S !== nothing
if !isdefined(F, :variable_indices) || coefficient_ring(S) !== codomain(F)
if !fl || 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))
return _evaluate_with_build_ctx(tmp_poly, var_ind, codomain(F))
end
else
if !isdefined(F, :variable_indices)
if !fl
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,
var_ind,
codomain(F)
)
# Otherwise default to the standard evaluation for the time being.
Expand Down