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

numargs fails for a function with explicit generic (and constrained) arguments #55

Closed
mobius-eng opened this issue Jul 11, 2017 · 8 comments

Comments

@mobius-eng
Copy link

I have a function with explicitly stated constrained generic arguments:

function equation!{T<:Real, U <:AbstractUnsaturatedModel{T}, V <: Inlet{T}}(t :: T, s :: Vector{T}, d :: Darcy{T,U,V}, Ds :: Vector{T})

Creating a parameterized function as

ParameterizedFunction(equation!, d)

Results in error in numargs (utils.jl) function that seems unable to calculate correctly the number of arguments of such a generic function. The problem is in the call

m.sig.parameters

where m is a method of equation!: there is no field parameters for my function.

For something simpler it works:

function lz(t, u, du)
	σ = 10.0
	ρ = 28.0
	β = 8/3.0
	du[1] = σ*(u[2]-u[1])
	du[2] = u[1]*-u[3]) - u[2]
	du[3] = u[1]*u[2] - β*u[3]
end

first(methods(lz)).sig.parameters
svec(DarcyExample.#lz, Any, Any, Any)

However the change to explicit argument type results in error:

function lz1{T<:Real}(t::T, u::Vector{T}, du::Vector{T})
	σ = 10.0
	ρ = 28.0
	β = 8/3.0
	du[1] = σ*(u[2]-u[1])
	du[2] = u[1]*-u[3]) - u[2]
	du[3] = u[1]*u[2] - β*u[3]
end

first(methods(lz1)).sig.parameters
type UnionAll has no field parameters
@ChrisRackauckas
Copy link
Member

Huh, thanks for catching this. I am not currently aware of a solution though... you can pass iip = true as a keyword arg to fix this though. That'll change with #51 to a different syntax ODEProblem{true}(f,u0,tspan), but hopefully a better fix for this can be found.

@ScottPJones do you know how to handle this at all?

@mauro3
Copy link
Contributor

mauro3 commented Jul 11, 2017

@ChrisRackauckas
Copy link
Member

Ahh yes, thanks @mauro3. Just noticed

julia> first(methods(lz1)).sig
Tuple{#lz1,T,Array{T,1},Array{T,1}} where T<:Real

julia> typeof(first(methods(lz1)).sig)
UnionAll

UnionAll means it was introduced in v0.6, and so I assume that JuliaLang/julia#22687 is precisely what we need. Hmm, I am not sure if there's a good v0.6 workaround.

julia> fieldnames(first(methods(lz1)).sig)
2-element Array{Int64,1}:
 1
 2

julia> first(methods(lz1)).sig
Tuple{#lz1,T,Array{T,1},Array{T,1}} where T<:Real

julia> first(methods(lz1)).sig.1
ERROR: syntax: extra token "0.1" after end of expression

julia> first(methods(lz1)).sig.2
ERROR: syntax: extra token "0.2" after end of expression

It seems rather peculiar.

@ChrisRackauckas
Copy link
Member

I think that means #51 plus #52 is likely a good intermediate fix, and this issue then fixes itself on v0.7/v1.0 unless we find someone who knows a hack around it.

@mobius-eng
Copy link
Author

@ChrisRackauckas I am on the master branch of DifferentialEquations.jl and ODEProblem does accept iip=true but ParameterizedFunction doesn't:

function lz(t, u, p, du)
	σ, ρ, β = p
	du[1] = σ*(u[2]-u[1])
	du[2] = u[1]*-u[3]) - u[2]
	du[3] = u[1]*u[2] - β*u[3]
end

flz = ParameterizedFunction(lz, (10.0, 28.0, 8/3.0),iip=true)
MethodError: no method matching DiffEqBase.ParameterizedFunction(::DarcyExample.#lz, ::Tuple{Float64,Float64,Float64}; iip=true)
Closest candidates are:
  DiffEqBase.ParameterizedFunction(::Any, ::Any) at /home/alexey/.julia/v0.6/DiffEqBase/src/constructed_parameterized_functions.jl:7 got unsupported keyword argument "iip"

and the error with the elaborate typing happens, in this case, at the call to ParameterizedFunction.

@ChrisRackauckas
Copy link
Member

I see. That part wasn't exposed. Try master now: e6a5b02 . I'll add this to the things which will be affected by #51

@ChrisRackauckas
Copy link
Member

Just confirmed that the latest example works on DiffEqBase master. So you can manually add iip to fix. As for the general case on v0.6, we should probably take this to Discourse to see if anyone has a v0.6 workaround for the auto-detection.

@ChrisRackauckas
Copy link
Member

@mobius-eng note that on the current DiffEqBase.jl that syntax changes to:

flz = ParameterizedFunction{true}(lz, (10.0, 28.0, 8/3.0))

All of the constructors which have to deal with inplace vs not-in-place have this setup now. It'll get documented ASAP. The advantage is that this is an inferrable way to specify it.

Additionally,

length(Base.unwrap_unionall(Tuple{T,T,T2} where {T,T2}).parameters)

works. So that means

length(Base.unwrap_unionall(first(methods(lz1)).sig).parameters)

will work for your case. Using this your original issue is now fixed on master.

function equation!{T<:Real, U , V}(t:: T, s :: Vector{T}, d :: U, Ds :: Vector{V}) end
ParameterizedFunction(equation!, [0.5])

I'll tag this soon.

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

3 participants