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

using cfunction on a callback wrapper #2700

Closed
dmbates opened this issue Mar 27, 2013 · 13 comments
Closed

using cfunction on a callback wrapper #2700

dmbates opened this issue Mar 27, 2013 · 13 comments

Comments

@dmbates
Copy link
Member

dmbates commented Mar 27, 2013

In the "Revised Optimization Functions" thread on julia-dev @stevengj blocked out how to wrap a closure as a callback function for the C optimizer code in the NLopt package. The (slightly modified) wrapper function is

function nlopt_callback_wrapper(n::Uint32, x::Ptr{Cdouble}, grad::Ptr{Cdouble}, f_::Ptr{Void})
     f = unsafe_pointer_to_objref(f_)::Function
     if grad == C_NULL return float(f(pointer_to_array(x, (n,)))) end
     float(f(pointer_to_array(x, (n,)), pointer_to_array(grad, (n,))))
end 

but the call to cfunction in

function set_min_objective!(opt::Opt, f::Function)
    status = ccall((:nlopt_set_min_objective, :libnlopt), Cint,
                   (Ptr{Opt_s}, Ptr{Void}, Any),
                   &opt.c,
                   cfunction(nlopt_callback_wrapper, Cdouble,
                             (Uint32, Ptr{Cdouble}, Ptr{Cdouble}, Ptr{Void})),
                   f)
    if status != NLOPT_SUCCESS throw(nloptException(status)) end
end

produces

julia> set_min_objective!(opt, rosenbrock)
ERROR: cfunction: nlopt_callback_wrapper does not return
 in set_min_objective! at /home/bates/.julia/NLopt/src/NLopt.jl:76

It seems that after the compilation of the method in jl_function_ptr the return type is not a Float64. Any suggestions for a workaround? I can appreciate that the return type of f cannot be inferred but is there some way for me to be more explicit about the return type of a call to nlopt_callback_wrapper?

@dmbates
Copy link
Member Author

dmbates commented Mar 27, 2013

Although the code shown is from the NLopt.jl package sketch (at https://github.com/dmbates/NLopt.jl.git) the nature of the Opt type and the rosenbrock function are tangential to the issue.

@Keno
Copy link
Member

Keno commented Mar 27, 2013

Have you tried ever calling the function with dummy parameters. I have found that julia may already know if there is some error in your code (thus the function not returning, since an error is thrown).

@JeffBezanson
Copy link
Member

Bug. Not being able to infer the type of something is routine; it shouldn't make it think the function doesn't return.

@JeffBezanson
Copy link
Member

Oh nevermind --- the problem is that pointer_to_array only accepts tuples of Int, not Uint32, so nlopt_callback_wrapper does indeed raise an error.

@dmbates
Copy link
Member Author

dmbates commented Mar 27, 2013

@JeffBezanson Thank you.

@dmbates
Copy link
Member Author

dmbates commented Mar 27, 2013

I'm a little further along now but still not there. I have modified the callback wrapper to

function nlopt_callback_wrapper(n::Uint32, x::Ptr{Cdouble}, grad::Ptr{Cdouble}, f_::Ptr{Void})
    res::Float64 = zero(Float64)
    if f_ != C_NULL
        f = unsafe_pointer_to_objref(f_)::Function
        if grad == C_NULL
            res = f(pointer_to_array(x, (int(n),)))
        else
            res = f(pointer_to_array(x, (n,)), pointer_to_array(grad, (int(n),)))
        end
    end
    res
end 

so that it is legitimate to call it as

julia> NLopt.nlopt_callback_wrapper(uint32(2), convert(Ptr{Float64},ones(2)), convert(Ptr{Float64},zeros(2)), C_NULL)
0.0

However, now I get

julia> set_min_objective!(opt, rosenbrock)
ERROR: cfunction: return type of nlopt_callback_wrapper does not match
 in set_min_objective! at /home/bates/.julia/NLopt/src/NLopt.jl:83

It seems to me that despite my efforts to convince the type inference engine that it can't return anything other than a Float64 the return type ends up being unknown, or something like that.

@vtjnash
Copy link
Member

vtjnash commented Mar 27, 2013

I'm not certain if this fixes it, by this line still throws an error
res = f(pointer_to_array(x, (n,)), pointer_to_array(grad, (int(n),)))
the first (n,) needs to be (int(n),)

or just make n an int in the first place with the line n = int(n)

@vtjnash
Copy link
Member

vtjnash commented Mar 27, 2013

to make this work, it seems we also need to type annotate the return value, otherwise the return value (from inspection of the disassembly) appears to be Any
@JeffBezanson is this a flaw in type inference, or correct since the result of the anonymous function could be anything?

julia> function nlopt_callback_wrapper(n::Uint32, x::Ptr{Cdouble}, grad::Ptr{Cdouble}, f_::Ptr{Void})
           res::Float64 = zero(Float64)
           if f_ != C_NULL
               f = unsafe_pointer_to_objref(f_)::Function
               if grad == C_NULL
                   res = f(pointer_to_array(x, (int(n),)))
               else
                   res = f(pointer_to_array(x, (int(n),)), pointer_to_array(grad, (int(n),)))
               end
           end
           res::Float64
       end 
# methods for generic function nlopt_callback_wrapper
nlopt_callback_wrapper(n::Uint32,x::Ptr{Float64},grad::Ptr{Float64},f_::Ptr{None}) at none:2

julia> cfunction(nlopt_callback_wrapper, Float64,
                                    (Uint32, Ptr{Cdouble}, Ptr{Cdouble}, Ptr{Void})),
(Ptr{Void} @0x000000010176c750,)

julia> 

@stevengj
Copy link
Member

Yes, I also found in PyCall that I had to annotate the return type; see e.g. jl_function_call in PyCall/src/callback.jl. Sorry I forgot to mention this on julia-dev.

You probably also want to call convert(Float64, f(....)) on the result of calling f to handle the case where the user's f returns some other Real type.

@JeffBezanson
Copy link
Member

@vtjnash this is an instance of issue #271 .

@amitmurthy
Copy link
Contributor

Shall I submit a documentation patch stating that type annotation on the return value in the callback is preferred?

Unless support for declaring the type of a function is coming soon, I feel that the above must be documented.

I just spent a good 20 minutes trying to figure out why I was getting a return type does not match error before I found this issue.

@jiahao
Copy link
Member

jiahao commented Apr 19, 2013

+1 for documentation

@ViralBShah
Copy link
Member

Please do.

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

No branches or pull requests

8 participants