From 472ed4521ee11d68dca69d17bbfe0a9a6a4b3ca5 Mon Sep 17 00:00:00 2001 From: Neven Sajko Date: Wed, 14 Dec 2022 17:17:51 +0100 Subject: [PATCH] deflate_leading_zeros fixes (#11) * fix deflate_leading_zeros zero polynomial handling There were multiple issues with the deflate_leading_zeros code for the case of a zero polynomial (where all vector elements are zero). The condition (`N == 0`) was wrong, a check for nothingness is needed instead as findfirst, findlast return `nothing` when nothing is found. This resulted in a useless error message from `deflate_leading_zeros` when `roots` was called with a zero polynomial. The second return value, whose definition isn't clear, seems like it was wrong for the zero polynomial case, judging by the fact that it depended on the number of zero coefficients, even though that number is irrelevant if all coefficients are zero. I also changed `zeros(S, 0)` to the simpler `S[]`. Regarding the replaced condition (`N == 0`), note that logically it would have been OK to replace it with either `isnothing(N)` or `isnothing(K)`, but I instead used `isnothing(N) | isnothing(K)` to help Julia with its type inference for the succeeding part of the function. * prevent run time dispatch in deflate_leading_zeros Seems Julia needs a bit of hand-holding here. Checked with development versions of Julia and JET. --- src/utils.jl | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/utils.jl b/src/utils.jl index 29a7d23..b0c8ee7 100644 --- a/src/utils.jl +++ b/src/utils.jl @@ -1,9 +1,21 @@ +# Returns a tuple (none_found::Bool, (first::Int, last::Int)) +function find_first_last( + pr::P, + v::AbstractVector) where {P <: Function} + + first = findfirst(pr, v) + last = findlast(pr, v) + + (isnothing(first) | isnothing(last)) && return (true, (0, 0)) + + (false, (first::Int, last::Int)) +end + function deflate_leading_zeros(ps::Vector{S}) where {S} - ## trim any 0s from the end of ps - N = findlast(!iszero, ps) - K = findfirst(!iszero, ps) + (is_empty, (K, N)) = find_first_last(!iszero, ps) + + is_empty && return (S[], 0) - N == 0 && return(zeros(S,0), length(ps)) # XXX should we make a view? ps = ps[K:N] ps, K-1