Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Julia 1.9.0 compatibility #91

Closed
ordicker opened this issue May 28, 2023 · 5 comments · Fixed by #92
Closed

Julia 1.9.0 compatibility #91

ordicker opened this issue May 28, 2023 · 5 comments · Fixed by #92

Comments

@ordicker
Copy link
Contributor

Hi,

I tried to run ONNX.jl ] test on julia 1.9.0 and got this error:

Gemm: Error During Test at /home/dicker/workspace/ONNX.jl/test/saveload.jl:23
  Got exception outside of a @test
  MethodError: save_node!(::GraphProto{ValueInfoProto, ValueInfoProto, NodeProto, ValueInfoProto}, ::ONNX.OpConfig{:ONNX, typeof(Core.kwcall)}, ::Umlaut.Call{typeof(Core.kwcall)}) is ambiguous.
  
  Candidates:
    save_node!(g::GraphProto, ::ONNX.OpConfig{:ONNX, <:Union{typeof(Core.kwcall), typeof(ONNX.onnx_flatten)}}, op::Umlaut.Call)
      @ ONNX ~/workspace/ONNX.jl/src/save.jl:149
    save_node!(g::GraphProto, ::ONNX.OpConfig{:ONNX, <:Union{typeof(Core.kwcall), typeof(ONNX.maxpool)}}, op::Umlaut.Call)
      @ ONNX ~/workspace/ONNX.jl/src/save.jl:134
    save_node!(g::GraphProto, ::ONNX.OpConfig{:ONNX, <:Union{typeof(Core.kwcall), typeof(ONNX.conv)}}, op::Umlaut.Call)
      @ ONNX ~/workspace/ONNX.jl/src/save.jl:122
    save_node!(g::GraphProto, ::ONNX.OpConfig{:ONNX, <:Union{typeof(Core.kwcall), typeof(ONNX.onnx_gemm)}}, op::Umlaut.Call)
      @ ONNX ~/workspace/ONNX.jl/src/save.jl:109
    save_node!(g::GraphProto, ::ONNX.OpConfig{:ONNX, <:Union{typeof(Core.kwcall), typeof(ONNX.batch_norm)}}, op::Umlaut.Call)
      @ ONNX ~/workspace/ONNX.jl/src/save.jl:204
    save_node!(g::GraphProto, ::ONNX.OpConfig{:ONNX, <:Union{typeof(Core.kwcall), typeof(ONNX.onnx_gather)}}, op::Umlaut.Call)
      @ ONNX ~/workspace/ONNX.jl/src/save.jl:251
    save_node!(g::GraphProto, ::ONNX.OpConfig{:ONNX, <:Union{typeof(Core.kwcall), typeof(ONNX.onnx_unsqueeze)}}, op::Umlaut.Call)
      @ ONNX ~/workspace/ONNX.jl/src/save.jl:261
    save_node!(g::GraphProto, ::ONNX.OpConfig{:ONNX, <:Union{typeof(Core.kwcall), typeof(ONNX.onnx_split)}}, op::Umlaut.Call)
      @ ONNX ~/workspace/ONNX.jl/src/save.jl:272
    save_node!(g::GraphProto, ::ONNX.OpConfig{:ONNX, <:Union{typeof(Core.kwcall), typeof(ONNX.onnx_concat)}}, op::Umlaut.Call)
      @ ONNX ~/workspace/ONNX.jl/src/save.jl:289

I'm guessing that related to this.
I'm not sure how to approach it, can I have some help?

@dfdx
Copy link
Collaborator

dfdx commented May 28, 2023

It looks like Core.kwfunc() has been replaced with Core.kwcall(), possibly with different semantics. I will try to take a look at it later today.

@dfdx
Copy link
Collaborator

dfdx commented May 28, 2023

A quick note. The whole machinery around functions with keyword arguments has changed in Julia 1.9. It will take some time to understand how to bypass the change and what other parts of the package it may affect.

@ToucheSir
Copy link
Member

I think kwfunc just returns kwcall now? So that still works as long as you don't rely on distinct kwsorters being returned for different functions.

@dfdx
Copy link
Collaborator

dfdx commented May 29, 2023

as long as you don't rely on distinct kwsorters being returned for different functions.

...which is pretty much what we do 😄

To be more precise, we dispatch calls between methods of save_node!() based on function type encoded into OpConfig. For simple functions we use:

OpConfig{backend, typeof(f)}
# e.g. 
OpConfig{:ONNX, typeof(*)}

For functions with keyword arguments we extend the config with the kw-based version of the function, e.g.

OpConfig{backend, Union{typeof(f), typeof(kwfunc(f))}}
# e.g. 
OpConfig{:ONNX, Union{typeof(conv), typeof(var"#conv##kw")}}

This way a single function save_node!(g::GraphProto, ::@opconfig_kw(:ONNX, conv), op::Umlaut.Call) can catch call to conv with and without kw arguments.


In Julia 1.9, however, kwfunc(f) returns not a unique function like var"#conv##kw", but instead a completely generic kwcall, on which we cannot dispatch. In #92 I try to mitigate it by always dispatching on the "main" function, e.g. conv, not var"#conv##kw", but there seems to be more errors on the way.

@dfdx
Copy link
Collaborator

dfdx commented May 29, 2023

Ok, the remaining errors were actually easy to fix. Now it works both - in Julia 1.8 and Julia 1.9. The code (see #92) is quite ugly though, so I will leave the PR open for a day to collect ideas on how to handle multiple Julia versions better.

@dfdx dfdx closed this as completed in #92 May 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants