Fixes for julia v1.10 #109

IanButterworth · 2023-04-25T19:23:38Z

Proposed fix for #108

Includes the fix from #112 to try and get CI green

codecov-commenter · 2023-04-25T19:32:51Z

Codecov Report

Patch coverage: 36.36% and project coverage change: -0.34 ⚠️

Comparison is base (62d5f42) 74.98% compared to head (68b9fa5) 74.64%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #109      +/-   ##
==========================================
- Coverage   74.98%   74.64%   -0.34%     
==========================================
  Files          15       15              
  Lines        1419     1428       +9     
==========================================
+ Hits         1064     1066       +2     
- Misses        355      362       +7

Impacted Files	Coverage Δ
src/reflection/utils.jl	`90.43% <28.57%> (-4.01%)`	⬇️
src/eval.jl	`88.88% <50.00%> (-5.23%)`	⬇️
src/reflection/reflection.jl	`83.33% <50.00%> (-1.42%)`	⬇️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

vtjnash · 2023-04-25T20:54:54Z

src/reflection/utils.jl

-@static if VERSION < v"1.8.0-DEV.267"
+@static if VERSION >= v"1.10.0-DEV.870"
+  function replace_code_newstyle!(ci, ir, _)
+    return Core.Compiler.replace_code_newstyle!(ci, ir)


you also might need to allocate slottypes, if you didn't already do that

ci.slottypes isa Array || (ci.slottypes = [])

Co-Authored-By: Jameson Nash <vtjnash+github@gmail.com>

IanButterworth · 2023-04-26T16:04:24Z

src/reflection/reflection.jl

  end
  Base.Meta.partially_inline!(ci.code, [], method.sig, Any[sps...], 0, 0, :propagate)
  Meta(method, mi, ci, method.nargs, sps)
 end

 function invoke_tweaks!(ci::CodeInfo)
+  ci.slottypes = [:todo, ci.slottypess[1], :todo, ci.slottypes[2:end]...]


I didn't know what to put here where there is :todo

I've gone with

ci.slottypes = [typeof(invoke), ci.slottypess[1], Type, ci.slottypes[2:end]...]

IanButterworth · 2023-04-26T16:40:26Z

I think I've done all the obvious stuff here. Seems like it needs someone with more IRTools knowledge

IanButterworth · 2023-04-26T20:17:48Z

@aviatesk @vtjnash any idea what's going on here? I'm not familiar with IRTools at all

vtjnash · 2023-04-27T02:07:16Z

Oh, it is using APIs that Base doesn't have anymore and need to be converted to the new API

maleadt · 2023-05-02T16:22:34Z

Here's with those APIs fixed: master...maleadt:IRTools.jl:replace_code_newstyle_fix
@IanButterworth Can you merge that here?

Only a couple of test failures remain.

Co-Authored-By: Tim Besard <383068+maleadt@users.noreply.github.com>

IanButterworth · 2023-05-02T16:50:37Z

Compiler: Test Failed at /home/runner/work/IRTools.jl/IRTools.jl/test/compiler.jl:93
  Expression: mullify(prod, [5, 10]) == 15
   Evaluated: 50 == 15

Is because https://github.com/IanButterworth/IRTools.jl/blob/668530fb38ca20678e0934efb3b74d93d8711925/test/compiler.jl#L80 is returning as nothing because https://github.com/IanButterworth/IRTools.jl/blob/6666ab00735fd52b73161c7fdf8755b8c4501a9c/src/ir/wrap.jl#L157-L158 is returning nothing.

Might this be related to Jameson's comment here f401a95#r111498985 @maleadt ?

Also, Zygote still fails to load with this in its current form, no method matching iterate(::Nothing) so appears to suffer a similar issue

IanButterworth · 2023-05-02T18:05:09Z

Appears to be a world age issue

julia> @dynamo function mullify(a...)
         @show ir = IR(a...)
         ir == nothing && return
         ir = MacroTools.prewalk(ir) do x
           x isa GlobalRef && x.name == :(*) && return GlobalRef(Base, :+)
           return x
         end
         for (x, st) in ir
           isexpr(st.expr, :call) || continue
           ir[x] = xcall(Main, :mullify, st.expr.args...)
         end
         return ir
       end

julia> mullify(prod, [5, 10])
Tuple{Ts...} = Tuple{typeof(prod), Vector{Int64}}
m = IRTools.meta(Tuple{Ts...}) = nothing
ir = IR(a...) = nothing

julia> IRTools.meta(Tuple{typeof(prod), Vector{Int64}})
Metadata for prod(a::AbstractArray; dims, kw...) @ Base reducedim.jl:994

maleadt · 2023-05-03T08:05:43Z

Might this be related to Jameson's comment here f401a95#r111498985 @maleadt ?

No, that's unrelated.

The problem is that @dynamo is fundamentally broken. Or at least the cases where the dynamo looks up Julia IR, which now needs to be done with world ages in mind (i.e. passing the age to reflection functions and setting edges on the returned CodeInfo), like the mullify one above. Sadly, this also seems like the way most packages use @dynamo.

…hange.

maleadt · 2023-05-03T08:44:53Z

Updated my fork; it passes tests now.

IanButterworth · 2023-05-03T10:13:06Z

Great! Although Zygote still fails to precompile on https://github.com/FluxML/Zygote.jl/blob/2287a86d1692c0dfc568739a80ca909199f6adfe/src/precompile.jl#LL17C20-L17C20

But perhaps that shouldn't stop this being merged and released?

ERROR: LoadError: MethodError: no method matching iterate(::Nothing)

Closest candidates are:
  iterate(::Union{LinRange, StepRangeLen})
   @ Base range.jl:887
  iterate(::Union{LinRange, StepRangeLen}, ::Integer)
   @ Base range.jl:887
  iterate(::T) where T<:Union{Base.KeySet{<:Any, <:Dict}, Base.ValueIterator{<:Dict}}
   @ Base dict.jl:728
  ...

Stacktrace:
  [1] indexed_iterate(I::Nothing, i::Int64)
    @ Base ./tuple.jl:95
  [2] chain_rrule(::Zygote.ZygoteRuleConfig{Zygote.Context{false}}, ::typeof(Zygote.pow), ::Int64, ::Int64)
    @ Zygote ~/.julia/packages/Zygote/SuKWp/src/compiler/chainrules.jl:223 [inlined]
  [3] macro expansion
    @ Zygote ~/.julia/packages/Zygote/SuKWp/src/compiler/interface2.jl:0 [inlined]
  [4] _pullback(::Zygote.Context{false}, ::typeof(Zygote.pow), ::Int64, ::Int64)
    @ Zygote ~/.julia/packages/Zygote/SuKWp/src/compiler/interface2.jl:9
  [5] pullback(::Function, ::Zygote.Context{false}, ::Int64, ::Vararg{Int64})
    @ Zygote ~/.julia/packages/Zygote/SuKWp/src/compiler/interface.jl:44
  [6] pullback(::Function, ::Int64, ::Int64)
    @ Zygote ~/.julia/packages/Zygote/SuKWp/src/compiler/interface.jl:42
  [7] gradient(::Function, ::Int64, ::Vararg{Int64})
    @ Zygote ~/.julia/packages/Zygote/SuKWp/src/compiler/interface.jl:96
  [8] top-level scope
    @ ~/.julia/packages/Zygote/SuKWp/src/precompile.jl:17

maleadt · 2023-05-03T10:47:50Z

I pushed another fix to my branch.

Some initial fixes for Zygote.jl are in https://github.com/maleadt/Zygote.jl/tree/dev, depending on this PR. This makes Zygote precompile and makes a smoke test gradient succeed, but there's more instances where the generated world age isn't propagated to reflection methods (resulting in nothing results, or in code reflection cannot be used from generated functions errors). Those should be relatively easy to fix though, with the core fixes in place.
EDIT: actually, it was only a single call to which. I've pushed a commit.

IanButterworth · 2023-05-07T13:30:24Z

It would be good to get this released

maleadt · 2023-05-09T17:23:27Z

@IanButterworth Can you merge the latest version of my branch?

IanButterworth · 2023-05-09T17:45:10Z

I believe this branch is up to date with yours?

maleadt · 2023-05-10T05:54:53Z

Ah yes, the GitHub UI is confusing here.

Can we get this (squash-)merged and tagged then? @CarloLucibello @ToucheSir

This was referenced Apr 25, 2023

Zygote is broken on Julia 1.10 FluxML/Zygote.jl#1410

Closed

test master on latest nightly #110

Closed

code reflection cannot be used from generated functions on Julia 1.10 #111

Closed

vtjnash reviewed Apr 25, 2023

View reviewed changes

IanButterworth and others added 3 commits April 26, 2023 12:03

fix replace_code_newstyle! on 1.10

1850a5c

fix from FluxML#112

92f80db

Co-Authored-By: Jameson Nash <vtjnash+github@gmail.com>

add world to get_staged

eb8bfd5

IanButterworth force-pushed the ib/replace_code_newstyle_fix branch from 28fd6ae to eb8bfd5 Compare April 26, 2023 16:03

IanButterworth commented Apr 26, 2023

View reviewed changes

IanButterworth added 3 commits April 26, 2023 12:12

try some types

8d90f12

fix older get_staged

b6a1371

only set slotypes on 1.10

68b9fa5

initialize slottypes as a Vector{Any} if nothing

f9419c2

Adapt to change in generated functions.

f401a95

Co-Authored-By: Tim Besard <383068+maleadt@users.noreply.github.com>

IanButterworth changed the title ~~fix replace_code_newstyle! on 1.10~~ Fixes for julia v1.10 May 2, 2023

unmark test as broken on julia v1.10

6666ab0

IanButterworth force-pushed the ib/replace_code_newstyle_fix branch from 668530f to 6666ab0 Compare May 2, 2023 16:48

No need to use the generated world when constructing from IR.

185a1f2

maleadt added 4 commits May 3, 2023 10:16

Spoof the current world to make dynamos work without a breaking API c…

d278705

…hange.

Fix typo.

59b323c

Don't copy slottypes if they aren't set.

9553b04

Re-mark test broken.

2ba04f5

IanButterworth requested a review from vtjnash May 3, 2023 10:13

Generate stub for every non-CI.

8d5a4f9

ToucheSir mentioned this pull request May 3, 2023

Seems to be #265ing FluxML/Zygote.jl#22

Open

CarloLucibello merged commit 97e2843 into FluxML:master May 10, 2023

This was referenced May 10, 2023

make call to get IR object more direct #112

Closed

replace_code_newstyle broken on v1.10 #108

Closed

IanButterworth deleted the ib/replace_code_newstyle_fix branch May 10, 2023 10:20

gafter mentioned this pull request May 29, 2023

Reflection will not work in a generated function in julia 1.10 JuliaServices/TreeTransform.jl#3

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes for julia v1.10 #109

Fixes for julia v1.10 #109

IanButterworth commented Apr 25, 2023 •

edited

Loading

codecov-commenter commented Apr 25, 2023 •

edited

Loading

vtjnash Apr 25, 2023

IanButterworth Apr 26, 2023

IanButterworth Apr 26, 2023

IanButterworth commented Apr 26, 2023

IanButterworth commented Apr 26, 2023

vtjnash commented Apr 27, 2023

maleadt commented May 2, 2023

IanButterworth commented May 2, 2023 •

edited

Loading

IanButterworth commented May 2, 2023

maleadt commented May 3, 2023 •

edited

Loading

maleadt commented May 3, 2023

IanButterworth commented May 3, 2023

maleadt commented May 3, 2023 •

edited

Loading

IanButterworth commented May 7, 2023

maleadt commented May 9, 2023

IanButterworth commented May 9, 2023

maleadt commented May 10, 2023

Fixes for julia v1.10 #109

Fixes for julia v1.10 #109

Conversation

IanButterworth commented Apr 25, 2023 • edited Loading

codecov-commenter commented Apr 25, 2023 • edited Loading

Codecov Report

vtjnash Apr 25, 2023

Choose a reason for hiding this comment

IanButterworth Apr 26, 2023

Choose a reason for hiding this comment

IanButterworth Apr 26, 2023

Choose a reason for hiding this comment

IanButterworth commented Apr 26, 2023

IanButterworth commented Apr 26, 2023

vtjnash commented Apr 27, 2023

maleadt commented May 2, 2023

IanButterworth commented May 2, 2023 • edited Loading

IanButterworth commented May 2, 2023

maleadt commented May 3, 2023 • edited Loading

maleadt commented May 3, 2023

IanButterworth commented May 3, 2023

maleadt commented May 3, 2023 • edited Loading

IanButterworth commented May 7, 2023

maleadt commented May 9, 2023

IanButterworth commented May 9, 2023

maleadt commented May 10, 2023

IanButterworth commented Apr 25, 2023 •

edited

Loading

codecov-commenter commented Apr 25, 2023 •

edited

Loading

IanButterworth commented May 2, 2023 •

edited

Loading

maleadt commented May 3, 2023 •

edited

Loading

maleadt commented May 3, 2023 •

edited

Loading