Error handling pass and standardization

[jlperla]

Need to add in the error handling idiom with shortcuts to the introductory material.

• We first need to agree on the error handling idioms
• See https://github.com/QuantEcon/lecture-source-jl/blob/master/style.md#error-handling
• This applies largely to the use of fixedpoint and optimize to ensure that we never ignore the return types.
• But we can then plan out going through our own functions and returning a nothing/etc.

[arnavs]

@jlperla I think the idioms in the style guide (use || short-circuiting, don't blindly swallow what we get from solvers, return nothing when possible and otherwise throw() an error) make sense to me.

@Nosferican, if you agree, we can get started on this and loop in some RAs.

[Nosferican]

Can I get an example of the specific cases? I prefer short-circuiting syntax, it is not hard to get once it is explained and is super useful. If there is an API to get the all-clear is best to rely on the API.

res = optimize(f, x)
res.converged || throw(ConvergenceException())

or something like that.

[arnavs]

@Nosferican Just to confirm, I think Jesse is proposing that we use the short-circuiting. But he's proposing that, rather than just throw an error, we return nothing and handle it ourselves, e.g.

    converged(result) || return nothing

from the style guide.

[Nosferican]

I would have to see the full proposed style. Usually is best to break early rather than accumulate the errors unless the pipeline really benefits from a convenient error handling. For example, when parsing or web-scrapping it can be beneficial to return nothing or some value and address those cases afterwards in order to continue and safely get the remaining.

[arnavs]

From the style guide-

Error Handling

• Consistent naming of result
  • Call the results of optimizers/etc. result when possible
• Add converged, etc. with using into the namespace to make the code easier to read
  • And can safely ignore the conflicting method errors, until smarter method merging becomes possible.
• Can use the || idiom for error handling.
  • Eventually people will need to get used to it. But minimize its use outside of that case
• Never ignore errors from fixed-point or solvers. In the case below, we can just raise an error so it isn't ignored

using NLsolve
f(x) = x
x_iv = [1.0]
#f(x) = 1.1 * x # fixed-point blows

# BAD!
xstar = fixedpoint(f, x_iv).zero # assumes convergence
xsol = nlsolve(f, x_iv).zero # assumes convergence

# GOOD!
result = fixedpoint(f, x_iv)
converged(result) || error("Failed to converge in $(result.iterations) iterations")
xstar = result.zero

result = nlsolve(f, x_iv)
converged(result) || error("Failed to converge in $(result.iterations) iterations")
xsol = result.zero

• Handle errors by returning nothing. But if you won't handle, prefer to throw errors.

function g(a)
    f(x) = a * x # won't succeed if a > 1
    result = fixedpoint(f, [1.0])
    converged(result) || return nothing
    xstar = result.zero
    
    #do calculations...
    return xstar
end
val = g(0.8)
@show val == nothing
val = g(1.1)
@show val == nothing;

• Use similar patterns with the Optim and other libraries
  • Although there is (currently) an inconsistency in the usage of the minimum and maximum in Optim.

# GOOD: make it easier to use, even if there are a few method merge warnings
using Optim
using Optim: converged, maximum, maximizer, minimizer, iterations

# BAD
xmin = optimize(x-> x^2, -2.0, 1.0).minimum

# GOOD
result = optimize(x-> x^2, -2.0, 1.0)
converged(result) || error("Failed to converge in $(iterations(result)) iterations")
xmin = result.minimizer
result.minimum


# GOOD
f(x) = -x^2
result = maximize(f, -2.0, 1.0)
converged(result) || error("Failed to converge in $(iterations(result)) iterations")
xmin = maximizer(result)
fmax = maximum(result)

[Nosferican]

Seems good! Just an issue on val == nothing, that should either be isa(val, Nothing), val isa Nothing or val === nothing.

When using an anonymous function like x -> x^2, it might one of those cases abs2 seems worthwhile.

[arnavs]

@Nosferican since == falls back to === (and Nothing is a singleton type anyway), I think this is OK? I'd rather not have to explain the distinction between == and === if it can be avoided

[jlperla]

Yes, throw errors instead of nothing unless the code actually handles the nothing.

For the nothing, go back to see in the fundamental types what I told them about comparing nothing. We can adjust as required. If

[jlperla]

I think the idioms in the style guide (use || short-circuiting, don't blindly swallow what we get from solvers, return nothing when possible and otherwise throw() an error) make sense to me.

Just to be clear: the preference is for the other direction, for throwing a simple error, unless we are planning to handle the nothing

[Nosferican]

I think the singleton === isa comparison is in the docs, but it should be consistent with what the lectures state (lectures on it hopefully consistent with the Julia docs).

[jlperla]

I think we can hold off on a full conversion of this until after the delete. I fixed up the existing comparison to === but the real solution will be isnothing. I think we can consider helping with a Compat there https://github.com/JuliaLang/Compat.jl/issues/634 at some point.