Julia: Consistent name for absolute and relative tolerance

[gabrielgellner]

In Base there are two conventions for specifying the tolerance parameters:

quadgk and use stebz! use abstol/reltol convention

whereas isapprox uses atol/rtol

In JuliaLang projects like ODE.jl the abstol/reltol is followed
whereas IterativeSolvers.jl, Roots.jl uses the atol/rtol convention

This is such a common set of parameter names for adaptive/iterative/approximate numerical methods it would be really nice to have a consistent convention put forward by the Julia community to follow. As this will be breaking it would be nice, at a minimum, in the style guide so future projects can aim for consistency.

[timholy]

Doesn't have to be breaking. You can add a depwarn for keyword arguments you don't want to support anymore.

[gabrielgellner]

Given that this can be made less painful. If people are willing to consider this change (and we decide on which convention to move forward with). I am happy to do the related pull requests to get this done, once 0.5 is out!

[stevengj]

👍

I prefer the slightly more verbose abstol and reltol convention, but I don't have strong opinions as long as we are consistent.

[gabrielgellner]

I am +1 for the longer convention as well.

[JeffreySarnoff]

+1   abstol reltol

[StefanKarpinski]

💯 for abstol and reltol. It took me a long time to figure out what atol and rtol even stood for when they were introduced. abstol and reltol are obvious (to me).

[JeffreySarnoff]

what happens now? atol rtol need to be deprecated in favor abstol and reltol, it appears.
(not always easy to discern where impetus finds its consensus)

[gabrielgellner]

If this continues to get support, my plan is to make a quick announcement on the mailing list, and the affected projects mentioned to get some more feedback (though luckily it seems that reltol/abstol seems popular both in packages and in peoples preference). And then I will prepare the pull requests to do the deprecation and add the new keywords for each of the projects.

As I mentioned I plan on doing this after 0.5 is out as I imagine people are busy with the pressing issues for the release.

[stevengj]

I wouldn't bother with the mailing list. Just a deprecation message in isapprox for the old keywords, and issues filed with any packages using the old names.

[gabrielgellner]

So trying to put this together I realize it is not entirely clear to me how to change the keywords using the deprecation tools. I can't really do matching on keywords since if I give compatible layer with the atol/rtol version then it shadows the newer abstol/reltol version since keyword arguments don't effect dispatch. The way out of this that I messed around with this is to have a depwarn with an if statement checking if atol/rtol arguments where provided in the actual function definition. Or make a new _isapprox like name for the newer abstol/reltol version that gets called by the deprecate layer which we could just rename after the deprecation occurs.

Am I missing something simple?

[gabrielgellner]

to be entirely clear on what I mean, I have the following ugly hack to get the deprecation to work as I would expect

function isapprox(x::Number, y::Number; rtol=nothing, atol=nothing, abstol::Real=0, reltol::Real=rtoldefault(x,y))
    flag = false
    if rtol != nothing
        flag = true
        _rtol = rtol
    else
        _rtol = reltol
    end

    if atol != nothing
        flag = true
        _atol = atol
    else
        _atol = abstol
    end

    if flag
        depwarn("isapprox(x, y; atol=tol, rtol=tol) should be replaced with isapprox(x, y; abstol=tol, reltol=tol)", :isapprox)
    end
    return _isapprox(x, y; reltol=_rtol, abstol=_atol)
end

I'm sure the exact logic (ie using all the if statements and flag arguments) could be cleaned up, but I was wondering if something like this is needed to change the names of a keyword argument.

[stevengj]

Yeah, I think you need to do something along those lines.

[gabrielgellner]

I just noticed that Optim.jl uses abs_tol and rel_tol (ie in brent.jl) which makes things uglier (as far as unification goes).

[stevengj]

@gabrielgellner, it wouldn't hurt to file an issue there and cite this one.

[gabrielgellner]

@stevengj will do. I will also get to getting these changes into a PR for 0.6 now that things seem to have settled down.