Add several updates for bitrot and QA improvements (#31)

* Try to fix badge links in README * Update CompatHelper.yml * Update Aqua test for newer Aqua API * Fix Aqua complaints in Project.toml * Restrict some input types to `Integer` These are always integers anyway * Fix JET complaint Seems to have a problem with `log(::Complex)` in general. Looks like a bug, if so. We don't really fix it, but rather skip it. * Don't test nightly on CI because it's broke with JET * Remove non-API function for input `z == MathConstant.e` The removed function has an underscore prefix and is not called at all by the API function `lambertw`. This non-API function returns 1::Int, which makes no sense. If you convert `z` to `Float64`, then `1.0::Float64` is returned, which is just as much exactly one as `1::Int`. The API function intercepts the input ::MathConstant.e and converts it to float * Improve doc strings lambertw was is outdated Function for branchpoint is better documented. * Remove special case for argument ::AbstractIrrational I don't see that this code path was called by the API in any case. But curiously, codecov showed that this line was in fact covered by the test suite. The test suite in fact still passes, as expected, with this line removed. * Remove another obsolete code path for special inputs * Integers and Rationals are converted to floats at the API entry point. The removed non-API functions would add nothing if they were called. In any case, they are not. * Part of the code path for `abs(z::Complex) <= one_t/convert(T, MathConstants.e)` was not tested before. This commit adds a test * Add tests for uncovered code paths * Some paths for complex arguments were not tested. This adds tests * A redundant logic test was removed from code in one of these paths. * Test inverse functions I used `finv(lambertw)`, but it was not tested. I tried promoting the idea, but didn't push hard and got no comments. There is now a package implementing this idea. In any case, this adds a test for the existing, unexported, function. * Remove unnecessary type parameter in signature * Simplify function signature I may have argued that the complicated one was slightly more performant. If it is, it is not significant. * Remove unused loop variable * Add tests for complex args to hit paths in branch point expansion * Remove unneccesary use of `local` * Remove another unused, and in any case, unneccesary code path * Test string(Omega()) * Update codecov action * Remove commented out cruft * Rename unused loop var to underscore
JuliaMath · Oct 2, 2024 · b82574a · b82574a
1 parent 6b6e2c8
commit b82574a
Show file tree

Hide file tree

Showing 8 changed files with 79 additions and 65 deletions.
diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml
@@ -21,7 +21,7 @@ jobs:
           - '1.0' # oldest supported release
           - '1.6' # LTS
           - '1' # stable release
-          - 'nightly'
+#          - 'nightly' nightly is broken if JET appears in [extras] in Project.toml (AFAICT)
         os:
           - ubuntu-latest
           - macOS-latest
@@ -38,6 +38,8 @@ jobs:
       - uses: julia-actions/julia-buildpkg@v1
       - uses: julia-actions/julia-runtest@v1
       - uses: julia-actions/julia-processcoverage@v1
-      - uses: codecov/codecov-action@v3
+      - uses: codecov/codecov-action@v4
         with:
           files: lcov.info
+          token: ${{ secrets.CODECOV_TOKEN }}
+          fail_ci_if_error: false
diff --git a/.github/workflows/CompatHelper.yml b/.github/workflows/CompatHelper.yml
@@ -1,4 +1,3 @@
-
 name: CompatHelper
 on:
   schedule:
@@ -16,7 +15,7 @@ jobs:
         run: which julia
         continue-on-error: true
       - name: Install Julia, but only if it is not already available in the PATH
-        uses: julia-actions/setup-julia@v1
+        uses: julia-actions/setup-julia@v2
         with:
           version: '1'
           arch: ${{ runner.arch }}

diff --git a/Project.toml b/Project.toml
@@ -9,14 +9,16 @@ version = "0.4.7"
 IrrationalConstants = "92d709cd-6900-40b7-9082-c6be49f344b6"
 
 [compat]
+Aqua = ">= 0.8"
 IrrationalConstants = "0.2.1"
 julia = "1"
+JET = ">= 0.0"
+Test = ">= 0.0"
 
 [extras]
 Aqua = "4c88cf16-eb10-579e-8560-4a9242c79595"
 JET = "c3a54625-cd67-489e-a8e7-0a5a0ff4e31b"
 Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
 
-
 [targets]
 test = ["Test", "Aqua", "JET"]
diff --git a/README.md b/README.md
@@ -2,9 +2,9 @@
 ### Lambert W function and associated omega constant
 
 [![Build Status](https://github.com/JuliaMath/LambertW.jl/actions/workflows/CI.yml/badge.svg?branch=master)](https://github.com/JuliaMath/LambertW.jl/actions/workflows/CI.yml?query=branch%3Amaster)
-[![codecov.io](http://codecov.io/github/JuliaMath/LambertW.jl/coverage.svg?branch=master)](http://codecov.io/github/JuliaMath/LambertW.jl?branch=master)
+[![Codecov](https://codecov.io/gh/JuliaMath/LambertW.jl/branch/master/graph/badge.svg)](https://codecov.io/gh/JuliaMath/LambertW.jl)
 [![Aqua QA](https://raw.githubusercontent.com/JuliaTesting/Aqua.jl/master/badge.svg)](https://github.com/JuliaTesting/Aqua.jl)
-[![JET QA](https://img.shields.io/badge/JET.jl-%E2%9C%88%EF%B8%8F-%23aa4444)](https://github.com/aviatesk/JET.jl)
+[![](https://img.shields.io/badge/%F0%9F%9B%A9%EF%B8%8F_tested_with-JET.jl-233f9a)](https://github.com/aviatesk/JET.jl)
 
 ### lambertw
 

diff --git a/src/LambertW.jl b/src/LambertW.jl
@@ -28,7 +28,7 @@ IrrationalConstants.@irrational lambertwbranchpoint -0.367879441171442321595 -bi
 @doc """
     lambertbranchpoint::IrrationalConstants.IrrationalConstant
 
-The branchpoint of the branches `k = 0` and `k = -1`, `-1/e`.
+The branchpoint of the branches `k = 0` and `k = -1`, `-1/e` of the Lambert W function.
 
 # Example
 
@@ -41,24 +41,25 @@ julia> lambertw(LambertW.lambertwbranchpoint, -1)
 ```
 """ lambertwbranchpoint
 
-#@doc "hi" lambertbranchpoint
-
 ### Lambert W function
 
 """
-    lambertw(z::Complex{T}, k::V=0) where {T<:Real, V<:Integer}
-    lambertw(z::T, k::V=0) where {T<:Real, V<:Integer}
+    lambertw(z, k::Integer=0, maxits::Integer=1000)
+
+Compute the `k`th branch of the Lambert W function of `z`.
 
-Compute the `k`th branch of the Lambert W function of `z`. If `z` is real, `k` must be
-either `0` or `-1`. For `Real` `z`, the domain of the branch `k = -1` is `[-1/e, 0]` and the
-domain of the branch `k = 0` is `[-1/e, Inf]`. For `Complex` `z`, and all `k`, the domain is
-the complex plane.
+If `z` is real, `k` must be either `0` or `-1`. For `Real` `z`, the domain of the branch
+`k = -1` is `[-1/e, 0]` and the domain of the branch `k = 0` is `[-1/e, Inf]`. For
+`Complex` `z`, and all `k`, the domain is the complex plane.
 
+The result is computed via a root-finding loop. If the number of iterations is greater
+than or equal to `maxits`, a warning is printed (or logged). In testing, this has never
+been observed.
 ```jldoctest
-julia> lambertw(-1/e, -1)
+julia> lambertw(-1/MathConstants.e, -1)
 -1.0
 
-julia> lambertw(-1/e, 0)
+julia> lambertw(-1/MathConstants.e, 0)
 -1.0
 
 julia> lambertw(0, 0)
@@ -74,19 +75,6 @@ julia> lambertw(Complex(-10.0, 3.0), 4)
 lambertw(z, k::Integer=0, maxits::Integer=1000) = _lambertw(float(z), k, maxits)
 
 # lambertw(e + 0im, k) is ok for all k
-# Maybe this should return a float. But, this should cause no type instability in any case
-function _lambertw(::typeof(MathConstants.e), k, maxits)
-    k == 0 && return 1
-    throw(DomainError(k))
-end
-_lambertw(x::AbstractIrrational, k, maxits) = _lambertw(float(x), k, maxits)
-function _lambertw(x::Union{Integer, Rational}, k, maxits)
-    if k == 0
-        x == 0 && return float(zero(x))
-        x == 1 && return convert(typeof(float(x)), LambertW.omega) # must be a more efficient way
-    end
-    return _lambertw(float(x), k, maxits)
-end
 
 ### Real z
 
@@ -130,11 +118,9 @@ end
 
 ### Complex z
 
-_lambertw(z::Complex{<:Integer}, k, maxits) = _lambertw(float(z), k, maxits)
 # choose initial value inside correct branch for root finding
-function _lambertw(z::Complex{T}, k, maxits) where T<:Real
+function _lambertw(z::Complex{T}, k::Integer, maxits::Integer) where T<:Real
     one_t = one(T)
-    local w::Complex{T}
     pointseven = 7//10
     if abs(z) <= one_t/convert(T, MathConstants.e)
         if z == 0
@@ -147,7 +133,7 @@ function _lambertw(z::Complex{T}, k, maxits) where T<:Real
             w = complex(log(-real(z)), 1//10^7) # need offset for z ≈ -1/e.
         else
             w = log(z)
-            k != 0 ? w += complex(0, k * 2 * pi) : nothing
+            w += complex(0, k * 2 * pi)
         end
     elseif k == 0 && imag(z) <= pointseven && abs(z) <= pointseven
         w = abs(z+ 1//2) < 1//10 ? imag(z) > 0 ? complex(pointseven, pointseven) : complex(pointseven, -pointseven) : z
@@ -173,7 +159,7 @@ function lambertw_root_finding(z::T, x0::T, maxits) where T <: Number
     lastx = x
     lastdiff = zero(T)
     converged::Bool = false
-    for i in 1:maxits
+    for _ in 1:maxits
         ex = exp(x)
         xexz = x * ex - z
         x1 = x + 1
@@ -192,6 +178,9 @@ end
 
 ### Inverse of Lambert W function
 
+# I had an Idea promote the use of `finv`, maybe put it in Base.
+# Got zero interest. In the meantime a package has appeared to do this.
+# It might be a good idea to remove this and use the package... `InverseFunctions`.
 """
     finv(::typeof(lambertw)) -> Function
 
@@ -358,7 +347,7 @@ function branch_point_series(p, x)
 end
 
 # These may need tuning.
-function branch_point_series(p::Complex{T}, z) where T<:Real
+function branch_point_series(p::Complex{<:Real}, z)
     x = abs(z)
     x < 4e-11 && return wser3(p)
     x < 1e-5 && return wser7(p)
@@ -410,11 +399,10 @@ julia> convert(Float64, (lambertw(-BigFloat(1)/e + BigFloat(10)^(-18), -1) + 1))
     The loss of precision in `lambertw` is analogous to the loss of precision
     in computing the `sqrt(1-x)` for `x` close to `1`.
 """
-function lambertwbp(x::Number, k::Integer)
+function lambertwbp(x::Number, k::Integer=0)
     k == 0 && return _lambertw0(x)
     k == -1 && return _lambertwm1(x)
     throw(ArgumentError("expansion about branch point only implemented for k = 0 and -1."))
 end
-lambertwbp(x::Number) = _lambertw0(x)
 
 end #module
diff --git a/test/aqua_test.jl b/test/aqua_test.jl
@@ -24,8 +24,8 @@ end
     Aqua.test_ambiguities([LambertW, Core, Base])
 end
 
-@testset "aqua piracy" begin
-    Aqua.test_piracy(LambertW)
+@testset "aqua piracies" begin
+    Aqua.test_piracies(LambertW)
 end
 
 @testset "aqua project extras" begin

diff --git a/test/jet_test.jl b/test/jet_test.jl
@@ -8,7 +8,9 @@ const package_to_analyze = LambertW
 ## the report message. The second is the file it occurs in.
 ## Not very precise, but ok for now.
 const SKIP_MATCHES = [
-    #  ("type Nothing has no field den", "parameters.jl"),
+    # JET 0.9.9 has a problem with: log(z::Complex) (Oct 1, 2024)
+    # So we skip it.
+    ("invalid builtin function call", "LambertW.jl"),
 ]
 
 ## Skip reports for which return true

diff --git a/test/lambertw_test.jl b/test/lambertw_test.jl
@@ -102,6 +102,9 @@ end # @testset "LambertW"
     # not a domain error, but not implemented
     @test_throws ArgumentError lambertwbp(1, 1)
     @test_throws DomainError lambertw(.3, 2)
+
+    @test_throws DomainError lambertwbp(1.1)
+    @test_throws DomainError lambertwbp(complex(1.1))
 end
 
 @testset "branch point" begin
@@ -112,31 +115,32 @@ end
     if Int != Int32
         # Test double-precision expansion near branch point using BigFloats
         sp = precision(BigFloat)
-        z = BigFloat(1)/10^12
-        setprecision(2048)
-        for i in 1:300
-#            innerarg = z - 1 / big(MathConstants.e)
-            innerarg = z + lambertwbranchpoint
-            # branch k = 0
-            wo = lambertwbp(Float64(z))
-            xdiff = abs(-1 + wo - lambertw(innerarg))
-            if xdiff > 5e-16
-                @warn(Float64(z), " ", Float64(xdiff))
-            end
-            @test xdiff < 5e-16
-
-            #  branch k = -1
-            wo = lambertwbp(Float64(z), -1)
-            xdiff = abs(-1 + wo - lambertw(innerarg, -1))
-            if xdiff > 5e-16
-                @warn(Float64(z), " ", Float64(xdiff))
+        zinit = BigFloat(1)/10^12
+        for z in (zinit, complex(zinit))
+            setprecision(2048)
+            for _ in 1:300 # We break from loop long before 300
+                #            innerarg = z - 1 / big(MathConstants.e)
+                innerarg = z + lambertwbranchpoint
+                # branch k = 0
+                wo = lambertwbp(Float64(z))
+                xdiff = abs(-1 + wo - lambertw(innerarg))
+                if xdiff > 5e-16
+                    @warn(Float64(z), " ", Float64(xdiff))
+                end
+                @test xdiff < 5e-16
+
+                #  branch k = -1
+                wo = lambertwbp(Float64(z), -1)
+                xdiff = abs(-1 + wo - lambertw(innerarg, -1))
+                if xdiff > 5e-16
+                    @warn(Float64(z), " ", Float64(xdiff))
+                end
+                @test xdiff < 5e-16
+                z  *= 1.1
+                if abs(z) > 0.23 break end
             end
-            @test xdiff < 5e-16
-            z  *= 1.1
-            if z > 0.23 break end
+            setprecision(sp)
         end
-        setprecision(sp)
-
         # test the expansion about branch point for k=-1,
         # by comparing to exact BigFloat calculation.
         @test lambertwbp(1e-20, -1) - 1 - lambertw(-BigFloat(1)/big(MathConstants.e)+ BigFloat(1)/BigFloat(10)^BigFloat(20), -1) < 1e-16
@@ -166,3 +170,20 @@ end
     @test k0val isa Float64
     @test km1val isa Float64
 end
+
+@testset "complex code paths" begin
+    z = complex(1/MathConstants.e - .01)
+    @test isapprox(lambertw(z), 0.27251232622985155)
+    @test isapprox(lambertw(z, -1), -2.6181060466381134 - 4.1495292474932475im)
+end
+
+@testset "finv" begin
+    lambertw_inv = LambertW.finv(lambertw)
+    z = 42.0
+    w = lambertw(z)
+    @test isapprox(z, lambertw_inv(w))
+end
+
+@testset "show" begin
+    @test string(LambertW.Omega()) == "ω"
+end