SciML / SciMLBase.jl

The Base interface of the SciML ecosystem
https://docs.sciml.ai/SciMLBase/stable
MIT License
131 stars 97 forks source link

Importing `SciMLBase` has side-effects on `Base.Math.throw_complex_domainerror` #606

Open tpgillam opened 8 months ago

tpgillam commented 8 months ago

Open a fresh julia session in a clean (temporary) environment. Then:

log(-1)
ERROR: DomainError with -1.0:
log was called with a negative real argument but will only return a complex result if called with a complex argument. Try log(Complex(x)).
Stacktrace:
 [1] throw_complex_domainerror(f::Symbol, x::Float64)
   @ Base.Math ./math.jl:33
 [2] _log(x::Float64, base::Val{:ℯ}, func::Symbol)
   @ Base.Math ./special/log.jl:301
 [3] log
   @ Base.Math ./special/log.jl:267 [inlined]
 [4] log(x::Int64)
   @ Base.Math ./math.jl:1578
 [5] top-level scope
   @ REPL[1]:1

Then,

using Pkg
Pkg.add("SciMLBase")
using SciMLBase

log(-1)
ERROR: DomainError with -1.0:
log was called with a negative real argument but will only return a complex result if called with a complex argument. Try log(Complex(x)).
DomainError detected in the user `f` function. This occurs when the domain of a function is violated.
For example, `log(-1.0)` is undefined because `log` of a real number is defined to only output real
numbers, but `log` of a negative number is complex valued and therefore Julia throws a DomainError
by default. Cases to be aware of include:

* `log(x)`, `sqrt(x)`, `cbrt(x)`, etc. where `x<0`
* `x^y` for `x<0` floating point `y` (example: `(-1.0)^(1/2) == im`)

Within the context of SciML, this error can occur within the solver process even if the domain constraint

<snip>

Stacktrace:
 [1] throw_complex_domainerror(f::Symbol, x::Float64)
   @ Base.Math ./math.jl:33
 [2] _log(x::Float64, base::Val{:ℯ}, func::Symbol)
   @ Base.Math ./special/log.jl:301
 [3] log
   @ Base.Math ./special/log.jl:267 [inlined]
 [4] log(x::Int64)
   @ Base.Math ./math.jl:1578
 [5] top-level scope
   @ REPL[13]:1

Because log is being called outside of the context of any SciML package, I would expect the normal error message to be thrown.

One thing that confuses me a bit is that, by behaviour, the Base.Math.throw_complex_domainerror function appears to have been piratically redefined:

Base.Math.throw_complex_domainerror(:moo, 1)
ERROR: DomainError with 1:
moo was called with a negative real argument but will only return a complex result if called with a complex argument. Try moo(Complex(x)).
DomainError detected in the user `f` function. This occurs when the domain of a function is violated.
For example, `log(-1.0)` is undefined because `log` of a real number is defined to only output real
numbers, but `log` of a negative number is complex valued and therefore Julia throws a DomainError
by default. Cases to be aware of include:

<snip>

... and yet Julia only claims that one method exists, with the standard implementation:

julia> methods(Base.Math.throw_complex_domainerror)
# 1 method for generic function "throw_complex_domainerror" from Base.Math:
 [1] throw_complex_domainerror(f::Symbol, x)
     @ math.jl:32

Even using e.g. @code_lowered Base.Math.throw_complex_domainerror(:moo, 1) shows the standard implementation, not what actually gets called.

Observed in Julia 1.10, and SciMLBase 2.21.0.

Additional context

This error caused a little confusion for me, as I assumed that it was coming from within a SciML function call -- in fact it was just a call to log in some other code I had written. But I happened to have an indirect dependency on SciMLBase, causing the above to occur.

ChrisRackauckas commented 8 months ago

There is no piracy since it's using the error hinting system. But I would like to figure out how to intercept the stack trace.

tpgillam commented 8 months ago

Thank you for the pointer - I hadn't come across the error hint system before.

It seems the current hinting API design could result in noise / confusion if several packages each registered their own hints for built-in exception types. (My opinion is that, to avoid this, no package should be able to register hints in this way, i.e. similar rules as for type piracy.)

But in any case, this is a comment about the underlying API, not this package- so feel free to close!

ChrisRackauckas commented 8 months ago

I'll keep it open and find out if I can inspect the stacktrace and make it only add if there's a SciML package in the trace.