@inferred
doesn’t seem to be sufficiently informative (especially in unit test sets), see e.g. https://github.com/JuliaLang/julia/issues/23371 : contrary to the error message @inferred
doesn’t show up on line 177 of test/statistics.jl, but at different places in the subsequent 68 lines.
I wrote a modified version of @inferred
that shows the expression for which the inference test failed (see infer.jl in https://gist.github.com/afniedermayer/9d3675e0a67ba140256120740127e6a8 ). Here’s what it does
julia> include("infer.jl")
julia> f(a,b,c) = b > 1 ? 1 : 1.0
julia> @inferred f(1,2,3)
ERROR: return type Int64 does not match inferred return type Union{Float64, Int64} in Expression f(1, 2, 3)
Stacktrace:
[1] error(::String) at .\error.jl:21
I can make a PR, but I’d like to have some feedback first.
(There’s an additional issue that unit testing is aborted if there is an @inferred
that fails, but I’m not sure what to do about that.)
1 Like
There’s the additional issue that @inferred
isn’t a unit test, so that
f(a,b,c) = b > 1 ? 1 : 1.0
@testset "foo" begin
@inferred f(1,2,3)
@test 2==3
@inferred f(1,2,4)
@test 4==5
end
will show only one failed unit test rather than four. Once https://github.com/JuliaLang/julia/issues/21594 gets fixed, it will show two rather than four failed unit tests, which is a step forward, but doesn’t go all the way.
Would it make sense to have an @isinferred
macro that returns a boolean? Here’s a simple (but not very elegant) implementation:
macro isinferred(ex)
quote try
@inferred $ex
true
catch err
println(err)
false
end
end
end
Usage:
@testset "foo" begin
@test @isinferred f(1,2,3)
@test 2==3
@test @isinferred f(1,2,4)
@test 4==5
end
This shows four failed tests and provides more informative messages about test failures.
3 Likes
This seems like an excellent feature to me – definitely a worthwhile improvement.
Not sure if this helps you, but we do this quite frequently:
julia> @testset "foo" begin
@test @inferred(f(1,2,3)) == 1
@test 2==3
@test @inferred(f(1,2,4)) == 1
@test 4==5
end
foo: Error During Test
Test threw an exception of type ErrorException
Expression: @inferred(f(1, 2, 3)) == 1
return type Int64 does not match inferred return type Union{Float64, Int64}
Stacktrace:
[1] macro expansion at ./REPL[7]:2 [inlined]
[2] macro expansion at ./test.jl:860 [inlined]
[3] anonymous at ./<missing>:?
foo: Test Failed
Expression: 2 == 3
Evaluated: 2 == 3
Stacktrace:
[1] macro expansion at ./REPL[7]:3 [inlined]
[2] macro expansion at ./test.jl:860 [inlined]
[3] anonymous at ./<missing>:?
foo: Error During Test
Test threw an exception of type ErrorException
Expression: @inferred(f(1, 2, 4)) == 1
return type Int64 does not match inferred return type Union{Float64, Int64}
Stacktrace:
[1] macro expansion at ./REPL[7]:4 [inlined]
[2] macro expansion at ./test.jl:860 [inlined]
[3] anonymous at ./<missing>:?
foo: Test Failed
Expression: 4 == 5
Evaluated: 4 == 5
Stacktrace:
[1] macro expansion at ./REPL[7]:5 [inlined]
[2] macro expansion at ./test.jl:860 [inlined]
[3] anonymous at ./<missing>:?
Test Summary: | Fail Error Total
foo | 2 2 4
ERROR: Some tests did not pass: 0 passed, 2 failed, 2 errored, 0 broken.
Thanks, @anon94023334. What’s good about @test @inferred(f(1,2,3)) == 1
is that it shows up as a unit test and doesn’t abort the testset when inference fails. But failure of inference shows up as “Error During Test” rather than “Test Failed”. You also have to check for the return value (which may be a good or a bad thing; bad if the return value is too complicated or random).
In julia/test out of the 124 times @inferred
is used, it is only in 35 cases that it is used in combination with @test
:
C:\Users\andras\git\julia\test>grep "@test.*@inferred " *.jl */*.jl | wc
35 295 2812
C:\Users\andras\git\julia\test>grep "@inferred " *.jl */*.jl | wc
124 714 7413
Some of the @inferred
s without @test
are using random numbers, they probably couldn’t be rewritten with the @test (@inferred f(1,2,3))==1
trick. Others probably could (should?) be rewritten that way.
I’ll open a PR adding @isinferred
once I have enough time.
Before I start implementing @isinferred
: is there a reason why @inferred
compares the actual return value with the inferred type ( https://github.com/JuliaLang/julia/blob/b056235d76d37f2dac309055b619b8f076c8ecb0/base/test.jl#L1175 ) rather than just checking whether the inferred type is abstract: !isleaftype(Base.return_type(...))[1]
? Would this miss some special cases?
The macro was originally written by @simonster and that part most recently tweaked by him as well, so perhaps he can shed some light on the issue.
I went ahead and started implementing @isinferred
with isleaf
. While trying to make sure that @isinferred
passes analogous tests to @inferred
, I ran into a bunch of problems with special cases (I guess there’s a reason so much tweaking had to go into @inferred
.)
In the end, I decided to simply call @inferred
from @isinferred
and catch exceptions.
Couldn’t you rename @inferred
to @isinferred
, change the exception throwing to returning false
and add a new @inferred
that does something like:
macro inferred (ex)
if !@isinferred($(esc(ex))
throw(...)
end
end
@inferred
also gives an error message with a description of the returned type and the inferred type. This couldn’t be recovered from a boolean.
Please package it up and possibly register that, I would start using this immediately.
@Tamas_Papp, I’m happy to hear that I just made a WIP pull request (I’m waiting for tests to pass on the server), but the code is already there and works, feel free to use it: https://github.com/JuliaLang/julia/pull/23426
I’m not sure that the code would warrant a separate package, because it’s relatively short. I spent most of the time documenting and making sure that unit tests pass.
2 Likes
I also created a package, since I created the additional macros @isinferred_noneval
and @return_types
that I didn’t want to put into the PR, but which might be useful for someone: https://github.com/afniedermayer/InferenceUtilities.jl
@isinferred
is also in the package.
The package is quite minimalistic and at an early stage, but it works.
2 Likes