Skip to content

Fixup #2196 #2221

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.

Sign up for GitHub

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

Merged
fingolfin merged 3 commits into from
Dec 1, 2025
Merged

Fixup #2196 #2221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
102ec7b
Fixup #2196
joschmitt Nov 20, 2025
356110e
Make `@inferred` check the right thing in two more places
joschmitt Nov 20, 2025
6a4051d
Add fallback
joschmitt Nov 24, 2025
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
4 changes: 4 additions & 0 deletions src/MPoly.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,10 @@ function exponent_vector!(e::Vector{S}, a::MPolyRingElem{T}, i::Int) where {T <:
return S.(exponent_vector(a, i))
end

function exponent_vector(::Type{Vector{S}}, a::MPolyRingElem{T}, i::Int) where {T <: RingElement, S}
return S.(exponent_vector(a, i))
end

function coeff!(c::T, a::MPolyRingElem{T}, i::Int) where {T <: RingElement}
return coeff(a, i)
end
Expand Down
12 changes: 8 additions & 4 deletions src/generic/MPoly.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,12 @@ are given in the order of the variables for the ring, as supplied when the
ring was created.
"""
function exponent_vector(a::MPoly{T}, i::Int) where T <: RingElement
e = Vector{Int}(undef, nvars(parent(a)))
return exponent_vector!(e, a, i)
return exponent_vector(Vector{Int}, a, i)
end

function exponent_vector(::Type{Vector{S}}, a::MPoly{T}, i::Int) where {T <: RingElement, S}
e = Vector{S}(undef, nvars(parent(a)))
Comment on lines +135 to +136
Copy link
Member

@fingolfin fingolfin Nov 27, 2025

Choose a reason for hiding this comment

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

This seems a bit unusual. I'd expect this to be either

Suggested change
function exponent_vector(::Type{Vector{S}}, a::MPoly{T}, i::Int) where {T <: RingElement, S}
e = Vector{S}(undef, nvars(parent(a)))
function exponent_vector(::Type{S}, a::MPoly{T}, i::Int) where {T <: RingElement, S}
e = S(undef, nvars(parent(a)))

(one could also restrict S to e.g. S <: Vector or S <: AbstractVetor)

Or else

Suggested change
function exponent_vector(::Type{Vector{S}}, a::MPoly{T}, i::Int) where {T <: RingElement, S}
e = Vector{S}(undef, nvars(parent(a)))
function exponent_vector(::Type{S}, a::MPoly{T}, i::Int) where {T <: RingElement, S}
e = Vector{S}(undef, nvars(parent(a)))

(and then change the callers to pass in just Int resp. eltype(someVectorType)

Copy link
Collaborator Author

@joschmitt joschmitt Nov 27, 2025

Choose a reason for hiding this comment

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

I intentionally did not implement the second option because there is/was this idea floating around that one should also be able to get the exponent vectors as ZZMatrix. Then I would find it inconsistent if there are exponent_vector(ZZRingElem, ...), which gives a Vector, and exponent_vector(ZZMatrix, ...).

I can implement the first variant. I might have preferred my version because it is easier to get the integer type that way, but I don't remember.

Copy link
Collaborator Author

@joschmitt joschmitt Nov 27, 2025

Choose a reason for hiding this comment

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

Now I'm wondering, is there a functional difference between ::Type{Vector{S}} where S and ::Type{S} where S <: Vector? As in, does one type get accepted by one but not the other?

Copy link
Member

@thofma thofma Nov 27, 2025

Choose a reason for hiding this comment

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

Not the same.

julia> f(::Type{Vector{S}}) where {S} = S
f (generic function with 1 method)

julia> g(::Type{S}) where {S <: Vector} = S
g (generic function with 1 method)

julia> f(Vector)
ERROR: MethodError: no method matching f(::Type{Vector})

Copy link
Collaborator Author

@joschmitt joschmitt Nov 27, 2025

Choose a reason for hiding this comment

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

Makes sense. That sounds to me like an argument not to change anything here?

Copy link
Member

@fingolfin fingolfin Dec 1, 2025

Choose a reason for hiding this comment

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

I don't think this suggests anything about whether f or g are "better" or "preferable".

Anyway, this point is not really important to me. Let's just get this merged?

return exponent_vector!(e, a, i)
end

function exponent_vector!(e::Vector{S}, a::MPoly{T}, i::Int) where {T <: RingElement, S}
Expand Down Expand Up @@ -843,10 +847,10 @@ function Base.iterate(x::MPolyCoeffs, state::Union{Nothing, Int} = nothing)
end
end

function Base.iterate(x::MPolyExponentVectors, state::Union{Nothing, Int} = nothing)
function Base.iterate(x::MPolyExponentVectors{T, S}, state::Union{Nothing, Int} = nothing) where {T, S}
s = isnothing(state) ? 1 : state + 1
if length(x.poly) >= s
v = x.inplace ? exponent_vector!(x.temp, x.poly, s) : exponent_vector(x.poly, s)
v = x.inplace ? exponent_vector!(x.temp, x.poly, s) : exponent_vector(S, x.poly, s)
return v, s
else
return nothing
Expand Down
1 change: 1 addition & 0 deletions src/generic/imports.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ import ..AbstractAlgebra: domain
import ..AbstractAlgebra: elem_type
import ..AbstractAlgebra: evaluate
import ..AbstractAlgebra: exp
import ..AbstractAlgebra: exponent_vector
import ..AbstractAlgebra: exponent_vectors
import ..AbstractAlgebra: exponent_vector!
import ..AbstractAlgebra: expressify
Expand Down
4 changes: 2 additions & 2 deletions test/Solve-test.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,8 @@ end

@test base_ring(MT) == QQ

@test @inferred zero(MT) == AbstractAlgebra.Solve.lazy_transpose(zero_matrix(QQ, 3, 5))
@test @inferred zero(MT, 2, 3) == AbstractAlgebra.Solve.lazy_transpose(zero_matrix(QQ, 3, 2))
@test (@inferred zero(MT)) == AbstractAlgebra.Solve.lazy_transpose(zero_matrix(QQ, 3, 5))
@test (@inferred zero(MT, 2, 3)) == AbstractAlgebra.Solve.lazy_transpose(zero_matrix(QQ, 3, 2))

S = @inferred similar(MT)
@test S isa AbstractAlgebra.Solve.LazyTransposeMatElem
Expand Down
22 changes: 11 additions & 11 deletions test/generic/MPoly-test.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1882,15 +1882,15 @@ end
R, (x, y, z) = polynomial_ring(QQ, [:x, :y, :z])
f = x * y + 2 * x - 3 * z

@test @inferred collect(exponent_vectors(f)) == [[1, 1, 0], [1, 0, 0], [0, 0, 1]]
@test @inferred collect(exponent_vectors(Vector{UInt}, f)) == [UInt[1, 1, 0], UInt[1, 0, 0], UInt[0, 0, 1]]
@test @inferred collect(coefficients(f)) == [QQ(1), QQ(2), QQ(-3)]
@test @inferred collect(terms(f)) == [x * y, 2 * x, -3 * z]
@test @inferred collect(monomials(f)) == [x * y, x, z]

@test @inferred first(exponent_vectors(f, inplace = true)) == [1, 1, 0]
@test @inferred first(exponent_vectors(Vector{UInt}, f, inplace = true)) == UInt[1, 1, 0]
@test @inferred first(coefficients(f, inplace = true)) == QQ(1)
@test @inferred first(monomials(f, inplace = true)) == x * y
@test @inferred first(terms(f, inplace = true)) == x * y
@test (@inferred collect(exponent_vectors(f))) == [[1, 1, 0], [1, 0, 0], [0, 0, 1]]
@test (@inferred collect(exponent_vectors(Vector{UInt}, f))) == [UInt[1, 1, 0], UInt[1, 0, 0], UInt[0, 0, 1]]
@test (@inferred collect(coefficients(f))) == [QQ(1), QQ(2), QQ(-3)]
@test (@inferred collect(terms(f))) == [x * y, 2 * x, -3 * z]
@test (@inferred collect(monomials(f))) == [x * y, x, z]

@test (@inferred first(exponent_vectors(f, inplace = true))) == [1, 1, 0]
@test (@inferred first(exponent_vectors(Vector{UInt}, f, inplace = true))) == UInt[1, 1, 0]
@test (@inferred first(coefficients(f, inplace = true))) == QQ(1)
@test (@inferred first(monomials(f, inplace = true))) == x * y
@test (@inferred first(terms(f, inplace = true))) == x * y
end
Loading