Stream: helpdesk (published)

Topic: Keeping JET happy


view this post on Zulip mbaz (May 12 2023 at 21:42):

I'm starting to use JET to test one of my packages (Gaston), and it's already found a few places with subtly incorrect code :heart:

However, in other cases I'm struggling with false positives. For example, in

if x in ('a', 'b', 'c')

it complains that I may be using missing in a Boolean context (since in can return missing). However, I'm sure that x is never missing. How can I tell JET that? I tried

x::Char
if x in ('a', 'b', 'c')

but it doesn't work. On the other hand, these work:

if x::Char in ('a', 'b', 'c')
if in(x::Char, ('a', 'b', 'c'))

but the code becomes less readable IMO. What's the best way to proceed here?

view this post on Zulip mbaz (May 12 2023 at 22:07):

One more; JET complains that IJulia is not defined:

isdefined(Main, :IJulia) && Main.IJulia.inited

It doesn't seem to take into account that && shortcircuits.

view this post on Zulip jar (May 12 2023 at 22:47):

Can you put the x::Char at the place where x is defined?

view this post on Zulip mbaz (May 12 2023 at 22:52):

Yep! That works. x is actually the iteration variable in an outer for loop, but it turns out I can type assert it there. Thanks!

view this post on Zulip mbaz (May 12 2023 at 22:56):

This one has me completely stumped... I have pl::Vector{Pair} and count::Int:

for z in pl[count+1:end]

makes JET complain with:

   @ /home/miguel/rcs/jdev/Gaston/src/gaston_aux.jl:465 pl[count Gaston.:+ 1 Gaston.:(:) lastindex(pl)]
  │┌ @ array.jl:925 copyto!(X, firstindex(X), A, first(I), lI)
  ││┌ @ array.jl:319 Base._copyto_impl!(dest, doffs, src, soffs, n)
  │││┌ @ array.jl:327 unsafe_copyto!(dest, doffs, src, soffs, n)
  ││││┌ @ array.jl:281 srcp = pointer(src, soffs)
  │││││┌ @ abstractarray.jl:1243 Base._memory_offset(x, i)
  ││││││┌ @ abstractarray.jl:1247 Base.elsize(x)
  │││││││┌ @ abstractarray.jl:251 Base.elsize(Base.typeof(A))
  ││││││││┌ @ array.jl:213 Base.aligned_sizeof(T)
  │││││││││┌ @ reflection.jl:392 al = Base.datatype_alignment(T)
  ││││││││││ no matching method found `datatype_alignment(::Type{Pair})`: al = Base.datatype_alignment(T::Type{Pair})

Mind, this code works perfectly fine under normal usage.

view this post on Zulip mbaz (May 12 2023 at 23:05):

Ha, figured it out! This was inside a function f(pl::Vector{Pair}) but I had to declare it as f(pl::Vector{P}) where P <: Pair. Darn transitive (or whatever) types get me all the time :sweat_smile:

view this post on Zulip mbaz (May 12 2023 at 23:05):

The undefined symbol is the one I don't know how to fix yet

view this post on Zulip Sukera (May 13 2023 at 06:11):

JET.jl can't really see through the isdefined check - it doesn't know what exactly you're passing as the Symbol (it just sees the symbol), so that could be improved. Maybe open an issue at JET? Not sure where the improvement would need to be, either there or in the compiler to eliminate the UndefVarError case there..

view this post on Zulip mbaz (May 13 2023 at 14:33):

Yeah, I can see that it's quite difficult to determine statically that during execution no undefined symbol will be found.

view this post on Zulip Sukera (May 13 2023 at 14:36):

I think cases like yours where you explicitly want to check whether IJulia is loaded should be handled by package extensions with 1.9+

view this post on Zulip mbaz (May 13 2023 at 14:42):

I want to keep compatibility with LTS, but I'll look into package extensions. I'm not sure it will do the trick, though -- IJulia is not a dependency, and I need to know if it's both loaded and active.

view this post on Zulip Brian Chen (May 13 2023 at 15:00):

Does JET not provide a way to ignore a block of code?

view this post on Zulip mbaz (May 13 2023 at 15:03):

I just started using it yesterday... I haven't found a way to do that.

view this post on Zulip mbaz (May 13 2023 at 15:03):

But I just skimmed through the docs.

view this post on Zulip Sukera (May 13 2023 at 15:13):

Not that I know of, no

view this post on Zulip Brian Chen (May 13 2023 at 15:16):

Seems like a major omission then. No linter is perfect. Granted the model JET uses may preclude any easy ways of marking this

view this post on Zulip jar (May 13 2023 at 18:35):

If the JET user has higher standards than the code author, the user will want JET errors for code the author doesn't want them for. The user should get the final say on what JET reports.

view this post on Zulip mbaz (May 13 2023 at 18:50):

https://github.com/aviatesk/JET.jl/issues/516

view this post on Zulip mbaz (May 13 2023 at 18:51):

But the user can always run their own tests on the code, right?

view this post on Zulip jar (May 13 2023 at 18:53):

The user wants to use JET to evaluate the code.

view this post on Zulip mbaz (May 13 2023 at 18:54):

And I (as package author) want my tests to pass... my point is that the user can always provide their own JET setup to evaluate code to their standards, regardless of what my own tests do

view this post on Zulip jar (May 13 2023 at 18:56):

Yep

view this post on Zulip Brian Chen (May 13 2023 at 19:22):

JET analyses are also a moving target. Getting warnings where there weren't any before after just a version bump is bad UX for a linter, and most of them take great pains to avoid that. Otherwise I'm not sure people would bother using such tools on CI.

view this post on Zulip Brian Chen (May 13 2023 at 19:24):

Granted, that's a different issue than being able to disable linter diagnostics for a certain section of code.

view this post on Zulip mbaz (May 13 2023 at 19:47):

This is my first time using JET, I'm waiting to see how it goes... I mostly plan to run it before tagging a new version or after significant code changes (same for AQUA)

view this post on Zulip mbaz (May 13 2023 at 21:33):

Argh... I found another kind of false positive: checking if a method exists with hasmethod before calling it, JET complains if the method doesn't exist.
It seems like static-checking a dynamic language like Julia is going to be difficult.

view this post on Zulip jar (May 13 2023 at 21:42):

The hasmethod pattern isn't really any good anyway because it fails if there's internal dispatch.

view this post on Zulip jar (May 13 2023 at 21:51):

It's more flexible to just try the thing and have it return a success or failure value you can handle

view this post on Zulip mbaz (May 14 2023 at 00:54):

Would it better to use applicable?

view this post on Zulip jar (May 14 2023 at 01:27):

No those are equivalent

view this post on Zulip Mason Protter (May 14 2023 at 01:35):

In version 1.10, the compiler will know about hasmethod. Here's version 1.9:

julia> f(x::Int) = x + 1;

julia> g(x) = hasmethod(g, Tuple{typeof(x)});

julia> @code_typed g(1)
CodeInfo(
1 ─ %1 = $(Expr(:foreigncall, :(:jl_get_world_counter), UInt64, svec(), 0, :(:ccall)))::UInt64
│        invoke Base.to_tuple_type(Tuple{Int64}::Any)::Type{Tuple{Int64}}
│   %3 = $(Expr(:foreigncall, :(:jl_gf_invoke_lookup), Any, svec(Any, Any, UInt64), 0, :(:ccall), Tuple{typeof(g), Int64}, :(Base.nothing), :(%1), :(%1)))::Any
│   %4 = (%3 === Base.nothing)::Bool
│   %5 = Core.Intrinsics.not_int(%4)::Bool
└──      return %5
) => Bool

julia> @btime g(1)
  111.074 ns (2 allocations: 112 bytes)
true

and here is v1.10

julia> f(x::Int) = x + 1;

julia> g(x) = hasmethod(g, Tuple{typeof(x)});

julia> @code_typed g(1)
CodeInfo(
1 ─     return true
) => Bool

julia> @btime g(1)
  1.080 ns (0 allocations: 0 bytes)
true

view this post on Zulip jar (May 14 2023 at 01:37):

Here's an example
https://github.com/JuliaPreludes/Try.jl#side-notes-on-hasmethod-and-applicable-and-invoke

view this post on Zulip Mason Protter (May 14 2023 at 01:37):

On earlier versions of julia you can use Tricks.jl for this though

julia> using Tricks: static_hasmethod

julia> f(x::Int) = x + 1;

julia> g(x) = static_hasmethod(g, Tuple{typeof(x)});

julia> @code_typed g(1)
CodeInfo(
1 ─     return true
) => Bool

julia> @btime g(1)
  1.080 ns (0 allocations: 0 bytes)
true

Be warned, Jameson says this implementation is incorrect, but so far as I'm aware, he still hasn't actually shown us an example where it is incorrect or even explained in what way

view this post on Zulip mbaz (May 14 2023 at 01:39):

why is this stuff not documented?

view this post on Zulip Mason Protter (May 14 2023 at 01:40):

Why is what not documented?

view this post on Zulip mbaz (May 14 2023 at 01:40):

that hasmethod and applicable are not reliable

view this post on Zulip Mason Protter (May 14 2023 at 01:41):

They are reliable.

view this post on Zulip mbaz (May 14 2023 at 01:43):

If I understand @jar correctly, they can return incorrect results in some cases

view this post on Zulip Mason Protter (May 14 2023 at 01:43):

They had unreliable performance mostly because the devs don't want you using them to build interfaces, but then people made Tricks.jl so eventually Jameson relented and made them fast

view this post on Zulip mbaz (May 14 2023 at 01:43):

@Mason Protter In the examples you showed, what is the role of f?

view this post on Zulip jar (May 14 2023 at 01:44):

Well it depends which question you mean to ask hasmethod: if your question is the more limited "does this function have this method" yes it's reliable. If it's "will this call run without a method error", it's not reliable.

view this post on Zulip Mason Protter (May 14 2023 at 01:44):

mbaz said:

If I understand jar correctly, they can return incorrect results in some cases

No, Jar is just pointing out that just because a function has an applicable method, doesn't mean you won't hit an error further down the call stack

view this post on Zulip Mason Protter (May 14 2023 at 01:45):

mbaz said:

Mason Protter In the examples you showed, what is the role of f?

Some function that only has one method

view this post on Zulip Mason Protter (May 14 2023 at 01:45):

E.g. it gives false on Float64:

julia> @code_typed g(1.0)
CodeInfo(
1 ─     return true
) => Bool

julia> @btime g(1.0)
  1.080 ns (0 allocations: 0 bytes)
true

view this post on Zulip Mason Protter (May 14 2023 at 01:46):

jar said:

Well it depends which question you mean to ask hasmethod: if your question is the more limited "does this function have this method" yes it's reliable. If it's "will this call run without a method error", it's not reliable.

To put it another way, hasmethod will give you the right answer, but you might still be asking a bad question.

view this post on Zulip jar (May 14 2023 at 01:47):

Yeah. I'd say the question that hasmethod can answer just isn't that useful a question.

view this post on Zulip mbaz (May 14 2023 at 01:48):

What about this case?

(v1.9) julia> f(x::Real) = x + 1
f (generic function with 1 method)

(v1.9) julia> hasmethod(f, Tuple{Float64})
true

but f has no method for tuples, right?

view this post on Zulip Mason Protter (May 14 2023 at 01:49):

hasmethod(f, Tuple{Float64}) is asking if f has a method for f(::Float64), not f(::Tuple{Float64}).

view this post on Zulip Mason Protter (May 14 2023 at 01:50):

I.e. hasmethod(f, Tuple{T, U, V}) is asking if f(::T, ::U, ::V) exists.

view this post on Zulip mbaz (May 14 2023 at 01:50):

Oh, darn, I forgot about hasmethod taking a tuple of types :face_palm:

view this post on Zulip Mason Protter (May 14 2023 at 01:50):

jar said:

Yeah. I'd say the question that hasmethod can answer just isn't that useful a question.

In general no, but for interfaces, yeah I'd say it can be useful.

view this post on Zulip mbaz (May 14 2023 at 01:50):

OK, I think I get it now.

view this post on Zulip Mason Protter (May 14 2023 at 01:51):

i.e. a classic one is hasmethod(iterate, Tuple{T}) to find out if you have an iterable.

view this post on Zulip mbaz (May 14 2023 at 01:52):

In my case, I just want to make sure a method exists for a certain combination of argument types.

view this post on Zulip jar (May 14 2023 at 01:54):

One way to find that out is to call the function with those arguments and see what happens

view this post on Zulip mbaz (May 14 2023 at 01:54):

With try/catch, right?

view this post on Zulip Mason Protter (May 14 2023 at 01:54):

yuck

view this post on Zulip mbaz (May 14 2023 at 01:55):

I don't love it

view this post on Zulip jar (May 14 2023 at 01:56):

What's the situation where you want to check whether a method exists?

view this post on Zulip mbaz (May 14 2023 at 01:58):

This is for my gnuplot frontend, Gaston. To simplify, let's say you can plot with plot(x, y). Gnuplot only understands real data in a certain format, so I need to convert whatever x and y are to this format.

view this post on Zulip mbaz (May 14 2023 at 01:59):

Gaston handles the usual data types, but I want to let users define their own conversions.

view this post on Zulip mbaz (May 14 2023 at 02:00):

So I provide a function convert_args for users to extend. So if users call plot(x::MyType, y::MyType), it works as long as users also provide convert_args(x::MyType, y::MyType) which returns data in gnuplot-compatible format

view this post on Zulip mbaz (May 14 2023 at 02:01):

Before calling convert_args, I check (with hasmethod) if such a method exists. This has been working fine for a long time, but JET flagged it.

view this post on Zulip jar (May 14 2023 at 02:02):

Why do you check instead of just letting it fail?

view this post on Zulip Mason Protter (May 14 2023 at 02:03):

Yeah, that seems like a case where I'd just let it fail

view this post on Zulip mbaz (May 14 2023 at 02:03):

Good question -- I guess the idea is to provide a nice error message explaining what the problem is and how to solve it.

view this post on Zulip Mason Protter (May 14 2023 at 02:04):

If you want to give them a good error message, you could always just do

convert_args(x, y) = error(ArgumentError("your types haven't yet implemented `convert_args` ..."))

view this post on Zulip mbaz (May 14 2023 at 02:05):

That looks indeed like a better solution, I think I'll go with it.

view this post on Zulip Mason Protter (May 14 2023 at 02:05):

Or even better, use Base.Experimental.register_error_hint.

view this post on Zulip Mason Protter (May 14 2023 at 02:06):

https://docs.julialang.org/en/v1/base/base/#Base.Experimental.register_error_hint

view this post on Zulip mbaz (May 14 2023 at 02:06):

Yeah, I was just looking it up

view this post on Zulip mbaz (May 14 2023 at 02:07):

Looks like it's worth a try

view this post on Zulip mbaz (May 14 2023 at 02:11):

@Mason Protter Coming back to the examples you posted above, g is defined as g(x) = hasmethod(g, Tuple{typeof(x)}), but shouldn't it be g(x) = hasmethod(f, Tuple{typeof(x)})?

view this post on Zulip Mason Protter (May 14 2023 at 02:12):

ha ha ha, yeah it should

view this post on Zulip Mason Protter (May 14 2023 at 02:12):

I should read what I write more carefully

view this post on Zulip mbaz (May 14 2023 at 02:13):

No problem :big_smile: I just wanted to make sure I understood what was going on

view this post on Zulip mbaz (May 14 2023 at 02:14):

BTW thanks a lot for your help @Mason Protter @jar

view this post on Zulip Sukera (May 15 2023 at 06:32):

arguably throw(ArgumentError(...)) is cleaner than error(ArgumentError(...)) :eyes:


Last updated: Nov 06 2024 at 04:40 UTC