-
Notifications
You must be signed in to change notification settings - Fork 126
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 more fixes #4142
Some more fixes #4142
Changes from 14 commits
accc8e1
93c4575
981ed63
b07ff91
4a085a1
843a1dc
34ac7ef
223c836
1c19d7f
6082101
320002a
783684d
2e9b228
c02dca0
211dc5c
2c94fb8
7d2622f
8866d53
59e12e1
e37061c
b69ce30
f8645db
cfd8e63
c2bea04
6545b39
c7f921c
e8cf31d
5052fa1
a467cd6
7c5e63b
7807c56
4173626
1a5e0e3
06ad700
6a10434
32b5883
fff3d65
afd68e1
67c4d72
fac1013
37bd37a
71503c8
67265a0
05e5518
1e4fb10
ca8ea64
6429108
dd08681
ebacd2e
8fe421c
183ecf4
4d7f64e
d93df25
cace0ba
63c1e3f
8ec2a8d
cc4a956
a03191c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -572,7 +572,23 @@ Ideal generated by | |
3*a*b*c | ||
``` | ||
""" | ||
@attr T function radical(I::T) where {T <: MPolyIdeal} | ||
@attr MPolyIdeal{T} function radical(I::MPolyIdeal{T}) where {T <: MPolyRingElem} | ||
is_known_to_be_radical(I) && return I | ||
# Calling `elimpart` (within `simplify`) turns out to significantly speed things up in many cases. | ||
R = base_ring(I) | ||
Q, pr = quo(R, I) | ||
S, iso, iso_inv = simplify(Q) | ||
is_zero(ngens(S)) && return I # This only happens when all variables can be eliminated. | ||
# Then the ideal defines a reduced point. | ||
J = modulus(S) | ||
pre_res = _compute_radical(J) | ||
pre_res = ideal(R, elem_type(R)[lift(iso_inv(S(g))) for g in gens(pre_res)]) | ||
res = pre_res + ideal(R, [g for g in gens(I) if !(g in pre_res)]) | ||
set_attribute!(res, :is_radical=>true) | ||
return res | ||
end | ||
|
||
function _compute_radical(I::T) where {T <: MPolyIdeal} | ||
R = base_ring(I) | ||
if isa(base_ring(R), NumField) && !isa(base_ring(R), AbsSimpleNumField) | ||
A, mA = absolute_simple_field(base_ring(R)) | ||
|
@@ -597,6 +613,7 @@ function radical( | |
I::MPolyIdeal{T}; | ||
factor_generators::Bool=true | ||
) where {U<:Union{AbsSimpleNumFieldElem, <:Hecke.RelSimpleNumFieldElem}, T<:MPolyRingElem{U}} | ||
is_known_to_be_radical(I) && return I | ||
get_attribute!(I, :radical) do | ||
is_one(I) && return I | ||
R = base_ring(I) | ||
|
@@ -629,7 +646,7 @@ end | |
|
||
function map_coefficients(mp, I::MPolyIdeal; parent = nothing) | ||
if parent === nothing | ||
parent = Oscar.parent(map_coefficients(mp, gen(I, 1))) | ||
parent = Oscar.parent(map_coefficients(mp, zero(base_ring(I)))) | ||
end | ||
return ideal(parent, [map_coefficients(mp, g, parent = parent) for g = gens(I)]) | ||
end | ||
|
@@ -642,12 +659,17 @@ Return whether `I` is a radical ideal. | |
Computes the radical. | ||
""" | ||
@attr Bool function is_radical(I::MPolyIdeal) | ||
if has_attribute(I, :is_prime) && is_prime(I) | ||
return true | ||
end | ||
has_attribute(I, :is_prime) && is_prime(I) && return true | ||
return is_subset(radical(I), I) | ||
end | ||
|
||
return I == radical(I) | ||
function is_known_to_be_radical(I::Ideal) | ||
has_attribute(I, :is_radical) && get_attribute(I, :is_radical) === true && return true | ||
has_attribute(I, :is_prime) && get_attribute(I, :is_prime) === true && return true | ||
return false | ||
end | ||
|
||
|
||
####################################################### | ||
@doc raw""" | ||
primary_decomposition(I::MPolyIdeal; algorithm = :GTZ, cache=true) | ||
|
@@ -1149,7 +1171,14 @@ function minimal_primes( | |
end | ||
J = K | ||
end | ||
result = unique!(filter!(!is_one, vcat([minimal_primes(j; algorithm, factor_generators=false) for j in J]...))) | ||
# unique! seems to fail here. We have to do it manually. | ||
pre_result = filter!(!is_one, vcat([minimal_primes(j; algorithm, factor_generators=false) for j in J]...)) | ||
result = typeof(I)[] | ||
for p in pre_result | ||
p in result && continue | ||
push!(result, p) | ||
end | ||
|
||
# The list might not consist of minimal primes only. We have to discard the embedded ones | ||
final_list = typeof(I)[] | ||
for p in result | ||
|
@@ -1209,7 +1238,7 @@ julia> L = equidimensional_decomposition_weak(I) | |
Ideal with 1 generator | ||
``` | ||
""" | ||
@attr Any function equidimensional_decomposition_weak(I::MPolyIdeal) | ||
@attr Vector{typeof(I)} function equidimensional_decomposition_weak(I::MPolyIdeal) | ||
R = base_ring(I) | ||
iszero(I) && return [I] | ||
@req coefficient_ring(R) isa AbstractAlgebra.Field "The coefficient ring must be a field" | ||
|
@@ -1235,7 +1264,7 @@ julia> L = equidimensional_decomposition_weak(I) | |
return V | ||
end | ||
|
||
@attr Any function equidimensional_decomposition_weak( | ||
@attr Vector{typeof(I)} function equidimensional_decomposition_weak( | ||
I::MPolyIdeal{T} | ||
) where {U<:Union{AbsSimpleNumFieldElem, <:Hecke.RelSimpleNumFieldElem}, T<:MPolyRingElem{U}} | ||
R = base_ring(I) | ||
|
@@ -2249,3 +2278,11 @@ function flag_pluecker_ideal(ring::MPolyRing{<: FieldElem}, dimensions::Vector{I | |
isReduced=true, | ||
isGB=true)) | ||
end | ||
|
||
# Since most ideals implement `==`, they have to implement the hash function. | ||
# See issue #4143 for problems entailed. Interestingly, this does not yet fix | ||
# the failure of unique! on lists of ideals. | ||
function hash(I::Ideal, c::UInt) | ||
return hash(typeof(I), hash(base_ring(I), c)) | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I advocate for this solution now. It is a matter of fact that ideals can not be hashed in general and even for polynomial rings this is probably expensive if not impossible (we need to choose an ordering which might be a dead end for a specific ideal). Yet, the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it safe to hash the type? After all two mathematically equal objects may have different types. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It is only safe if you don't implement There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, good to know. I was only following the suggestion by @benlorenz . But you're probably right. |
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
save some allocations