-
Notifications
You must be signed in to change notification settings - Fork 72
"Inplace" iterators over the coefficients, monomials, terms and exponent vectors of a multivariate polynomial #2196
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
Changes from 13 commits
ff7c58d
c4ef7ca
13bc030
7ebd24f
9100d4f
8bd71ef
c774ba0
49c5192
3057fdc
4a0ae6d
7d73418
ef3b593
d6e24a0
d62e1c5
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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -488,51 +488,87 @@ function is_monomial(x::MPolyRingElem{T}) where T <: RingElement | |||||
| return length(x) == 1 && isone(first(coefficients(x))) | ||||||
| end | ||||||
|
|
||||||
| function exponent_vector!(e::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 | ||||||
|
|
||||||
| function term!(t::T, a::T, i::Int) where {T <: MPolyRingElem} | ||||||
| return term(a, i) | ||||||
| end | ||||||
|
|
||||||
| function monomial!(m::T, a::T, i::Int) where {T <: MPolyRingElem} | ||||||
| return monomial(a, i) | ||||||
| end | ||||||
|
|
||||||
| ############################################################################### | ||||||
| # | ||||||
| # Iterators | ||||||
| # | ||||||
| ############################################################################### | ||||||
|
|
||||||
| @doc raw""" | ||||||
| coefficients(a::MPolyRingElem{T}) where T <: RingElement | ||||||
| coefficients(a::MPolyRingElem{T}; inplace::Bool = false) where T <: RingElement | ||||||
|
|
||||||
| Return an iterator for the coefficients of the given polynomial. To retrieve | ||||||
| an array of the coefficients, use `collect(coefficients(a))`. | ||||||
|
|
||||||
| If `inplace` is `true`, the elements of the iterator may share their memory. This | ||||||
| means that an element returned by the iterator may be overwritten 'in place' in | ||||||
| the next iteration step. This may result in significantly fewer memory allocations. | ||||||
| """ | ||||||
| function coefficients(a::MPolyRingElem{T}) where T <: RingElement | ||||||
| return Generic.MPolyCoeffs(a) | ||||||
| function coefficients(a::MPolyRingElem{T}; inplace::Bool = false) where T <: RingElement | ||||||
| return Generic.MPolyCoeffs(a, inplace=inplace) | ||||||
|
Member
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.
Suggested change
IMO a bit neater, but I don't really care
Collaborator
Author
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 never really got the hang of this syntax, so if you don't care, I'll prefer my version. |
||||||
| end | ||||||
|
|
||||||
| @doc raw""" | ||||||
| exponent_vectors(a::MPolyRingElem{T}) where T <: RingElement | ||||||
| exponent_vectors(a::MPolyRingElem{T}; inplace::Bool = false) where T <: RingElement | ||||||
|
|
||||||
| Return an iterator for the exponent vectors of the given polynomial. To | ||||||
| retrieve an array of the exponent vectors, use | ||||||
| `collect(exponent_vectors(a))`. | ||||||
|
|
||||||
| If `inplace` is `true`, the elements of the iterator may share their memory. This | ||||||
| means that an element returned by the iterator may be overwritten 'in place' in | ||||||
| the next iteration step. This may result in significantly fewer memory allocations. | ||||||
| """ | ||||||
| function exponent_vectors(a::MPolyRingElem{T}) where T <: RingElement | ||||||
| return Generic.MPolyExponentVectors(a) | ||||||
| function exponent_vectors(a::MPolyRingElem{T}; inplace::Bool = false) where T <: RingElement | ||||||
| return Generic.MPolyExponentVectors(a, inplace=inplace) | ||||||
| end | ||||||
|
|
||||||
| function exponent_vectors(::Type{Vector{S}}, a::MPolyRingElem{T}; inplace::Bool = false) where {T <: RingElement, S} | ||||||
| return Generic.MPolyExponentVectors(Vector{S}, a, inplace=inplace) | ||||||
| end | ||||||
|
|
||||||
| @doc raw""" | ||||||
| monomials(a::MPolyRingElem{T}) where T <: RingElement | ||||||
| monomials(a::MPolyRingElem{T}; inplace::Bool = false) where T <: RingElement | ||||||
|
|
||||||
| Return an iterator for the monomials of the given polynomial. To retrieve | ||||||
| an array of the monomials, use `collect(monomials(a))`. | ||||||
|
|
||||||
| If `inplace` is `true`, the elements of the iterator may share their memory. This | ||||||
| means that an element returned by the iterator may be overwritten 'in place' in | ||||||
| the next iteration step. This may result in significantly fewer memory allocations. | ||||||
| """ | ||||||
| function monomials(a::MPolyRingElem{T}) where T <: RingElement | ||||||
| return Generic.MPolyMonomials(a) | ||||||
| function monomials(a::MPolyRingElem{T}; inplace::Bool = false) where T <: RingElement | ||||||
| return Generic.MPolyMonomials(a, inplace=inplace) | ||||||
| end | ||||||
|
|
||||||
| @doc raw""" | ||||||
| terms(a::MPolyRingElem{T}) where T <: RingElement | ||||||
| terms(a::MPolyRingElem{T}; inplace::Bool = false) where T <: RingElement | ||||||
|
|
||||||
| Return an iterator for the terms of the given polynomial. To retrieve | ||||||
| an array of the terms, use `collect(terms(a))`. | ||||||
|
|
||||||
| If `inplace` is `true`, the elements of the iterator may share their memory. This | ||||||
| means that an element returned by the iterator may be overwritten 'in place' in | ||||||
| the next iteration step. This may result in significantly fewer memory allocations. | ||||||
| """ | ||||||
| function terms(a::MPolyRingElem{T}) where T <: RingElement | ||||||
| return Generic.MPolyTerms(a) | ||||||
| function terms(a::MPolyRingElem{T}; inplace::Bool = false) where T <: RingElement | ||||||
| return Generic.MPolyTerms(a, inplace=inplace) | ||||||
| end | ||||||
|
|
||||||
| ############################################################################### | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -125,19 +125,31 @@ 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) | ||
| end | ||
|
|
||
| function exponent_vector!(e::Vector{S}, a::MPoly{T}, i::Int) where {T <: RingElement, S} | ||
| @assert length(e) == nvars(parent(a)) | ||
| A = a.exps | ||
| N = size(A, 1) | ||
|
|
||
| ord = internal_ordering(parent(a)) | ||
| if ord == :lex | ||
| return [Int(A[j, i]) for j in N:-1:1] | ||
| range = N:-1:1 | ||
| elseif ord == :deglex | ||
| return [Int(A[j, i]) for j in N - 1:-1:1] | ||
| range = N - 1:-1:1 | ||
| elseif ord == :degrevlex | ||
| return [Int(A[j, i]) for j in 1:N - 1] | ||
| range = 1:N - 1 | ||
| else | ||
| error("invalid ordering") | ||
| end | ||
| k = 1 | ||
| for j in range | ||
| e[k] = S(A[j, i]) | ||
| k += 1 | ||
| end | ||
| return e | ||
| end | ||
|
|
||
| @doc raw""" | ||
|
|
@@ -635,6 +647,11 @@ function coeff(x::MPoly, i::Int) | |
| return x.coeffs[i] | ||
| end | ||
|
|
||
| # Only for compatibility, we can't do anything in place here | ||
| function coeff!(c::T, x::MPoly{T}, i::Int) where T <: RingElement | ||
| return x.coeffs[i] | ||
| end | ||
|
|
||
| function trailing_coefficient(p::MPoly{T}) where T <: RingElement | ||
| @req !iszero(p) "Zero polynomial does not have a leading monomial" | ||
| return coeff(p, length(p)) | ||
|
|
@@ -664,7 +681,9 @@ function monomial!(m::MPoly{T}, x::MPoly{T}, i::Int) where T <: RingElement | |
| N = size(x.exps, 1) | ||
| fit!(m, 1) | ||
| monomial_set!(m.exps, 1, x.exps, i, N) | ||
| m.coeffs[1] = one(base_ring(x)) | ||
| if !isassigned(m.coeffs, 1) || !is_one(m.coeffs[1]) | ||
| m.coeffs[1] = one(base_ring(x)) | ||
| end | ||
|
Comment on lines
+684
to
+686
Collaborator
Author
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. This is a bit ugly, but it gets the
Member
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. Seems fine to me |
||
| m.length = 1 | ||
| return m | ||
| end | ||
|
|
@@ -675,11 +694,17 @@ end | |
| Return the $i$-th nonzero term of the polynomial $x$ (as a polynomial). | ||
| """ | ||
| function term(x::MPoly, i::Int) | ||
| R = base_ring(x) | ||
| y = zero(parent(x)) | ||
| return term!(y, x, i) | ||
| end | ||
|
|
||
| function term!(y::T, x::T, i::Int) where T <: MPoly | ||
| N = size(x.exps, 1) | ||
| exps = Matrix{UInt}(undef, N, 1) | ||
| monomial_set!(exps, 1, x.exps, i, N) | ||
| return parent(x)([deepcopy(x.coeffs[i])], exps) | ||
| fit!(y, 1) | ||
| monomial_set!(y.exps, 1, x.exps, i, N) | ||
| y.coeffs[1] = deepcopy(x.coeffs[i]) | ||
| y.length = 1 | ||
| return y | ||
| end | ||
|
|
||
| @doc raw""" | ||
|
|
@@ -804,69 +829,41 @@ Base.copy(f::Generic.MPoly) = deepcopy(f) | |
| # | ||
| ############################################################################### | ||
|
|
||
| function Base.iterate(x::MPolyCoeffs) | ||
| if length(x.poly) >= 1 | ||
| return coeff(x.poly, 1), 1 | ||
| else | ||
| return nothing | ||
| end | ||
| end | ||
|
|
||
| function Base.iterate(x::MPolyCoeffs, state) | ||
| state += 1 | ||
| if length(x.poly) >= state | ||
| return coeff(x.poly, state), state | ||
| else | ||
| return nothing | ||
| end | ||
| end | ||
|
|
||
| function Base.iterate(x::MPolyExponentVectors) | ||
| if length(x.poly) >= 1 | ||
| return exponent_vector(x.poly, 1), 1 | ||
| else | ||
| return nothing | ||
| end | ||
| end | ||
|
|
||
| function Base.iterate(x::MPolyExponentVectors, state) | ||
| state += 1 | ||
| if length(x.poly) >= state | ||
| return exponent_vector(x.poly, state), state | ||
| else | ||
| return nothing | ||
| end | ||
| end | ||
|
|
||
| function Base.iterate(x::MPolyTerms) | ||
| if length(x.poly) >= 1 | ||
| return term(x.poly, 1), 1 | ||
| function Base.iterate(x::MPolyCoeffs, state::Union{Nothing, Int} = nothing) | ||
| s = isnothing(state) ? 1 : state + 1 | ||
| if length(x.poly) >= s | ||
| c = x.inplace ? coeff!(x.temp, x.poly, s) : coeff(x.poly, s) | ||
| return c, s | ||
| else | ||
| return nothing | ||
| end | ||
| end | ||
|
|
||
| function Base.iterate(x::MPolyTerms, state) | ||
| state += 1 | ||
| if length(x.poly) >= state | ||
| return term(x.poly, state), state | ||
| function Base.iterate(x::MPolyExponentVectors, state::Union{Nothing, Int} = nothing) | ||
| 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) | ||
| return v, s | ||
| else | ||
| return nothing | ||
| end | ||
| end | ||
|
|
||
| function Base.iterate(x::MPolyMonomials) | ||
| if length(x.poly) >= 1 | ||
| return monomial(x.poly, 1), 1 | ||
| function Base.iterate(x::MPolyTerms, state::Union{Nothing, Int} = nothing) | ||
| s = isnothing(state) ? 1 : state + 1 | ||
| if length(x.poly) >= s | ||
| t = x.inplace ? term!(x.temp, x.poly, s) : term(x.poly, s) | ||
| return t, s | ||
| else | ||
| return nothing | ||
| end | ||
| end | ||
|
|
||
| function Base.iterate(x::MPolyMonomials, state) | ||
| state += 1 | ||
| if length(x.poly) >= state | ||
| return monomial(x.poly, state), state | ||
| function Base.iterate(x::MPolyMonomials, state::Union{Nothing, Int} = nothing) | ||
| s = isnothing(state) ? 1 : state + 1 | ||
| if length(x.poly) >= s | ||
| m = x.inplace ? monomial!(x.temp, x.poly, s) : monomial(x.poly, s) | ||
| return m, s | ||
| else | ||
| return nothing | ||
| end | ||
|
|
@@ -876,12 +873,12 @@ function Base.length(x::Union{MPolyCoeffs, MPolyExponentVectors, MPolyTerms, MPo | |
| return length(x.poly) | ||
| end | ||
|
|
||
| function Base.eltype(::Type{MPolyCoeffs{T}}) where T <: AbstractAlgebra.MPolyRingElem{S} where S <: RingElement | ||
| function Base.eltype(::Type{MPolyCoeffs{T, S}}) where {T <: AbstractAlgebra.MPolyRingElem, S <: RingElement} | ||
| return S | ||
| end | ||
|
|
||
| function Base.eltype(::Type{MPolyExponentVectors{T}}) where T <: AbstractAlgebra.MPolyRingElem{S} where S <: RingElement | ||
| return Vector{Int} | ||
| function Base.eltype(::Type{MPolyExponentVectors{T, V}}) where {V, T <: AbstractAlgebra.MPolyRingElem{S} where S <: RingElement} | ||
| return V | ||
| end | ||
|
|
||
| function Base.eltype(::Type{MPolyMonomials{T}}) where T <: AbstractAlgebra.MPolyRingElem{S} where S <: RingElement | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
I added these sentences to the doc strings. I'm happy for any suggestions. If I understand the suggestion in oscar-system/Oscar.jl#5530 (comment) correctly, we want to have something to add to all iterator doc strings that allow
inplace. So maybe we should agree on a nice wording here and then we can copy and paste it everywhere :)@fingolfin @mjrodgers
EDIT: I kept this intentionally vague ("may") because with this generic code it might be that the inplace version actually doesn't do anything different (because something is not implemented for a particular type).
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.
I like this, if others agree I will add this same wording to the other methods in Oscar.
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.
Maybe mention that it might to lead to unexpected behavior when calling
collecton such an iterator.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.
I concur
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.
I added a bit more, so this is ready for the next round of feedback :)
I feel like in an ideal world, we would put this paragraph somewhere "central" in the documentation and just link there from all iterators that support
inplace = true. But I don't know where.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.
Looks good to me, thanks. (And I agree, one "central place" would be great, but for the time being this here is fine.)