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

Struct methods and keyword arguments don't appear to work properly #32

Closed
pepijndevos opened this issue May 8, 2023 · 2 comments
Closed

Comments

@pepijndevos
Copy link

I was having a bug where it seems like if I call a struct method it works, unless I pass keyword arguments, in which cases it just seems to bypass the overlay.

But then when I tried to debug it I got even more broken behavior:

using CassetteOverlay, Test

struct ParamInterp <: CassetteOverlay.AbstractBindingOverlay{nothing, nothing}
end

@Base.kwdef struct mystruct
    x::Int=1
    y::Int=2
end

function (::mystruct)(x=nothing; y=nothing)
    println("mystruct instance!!")
end

inst = mystruct(x=1, y=2)

function (self::ParamInterp)(fn::typeof(mystruct), args...; kwargs...)
    @show "cassette constructor!!", args, kwargs
    @show mystruct(; kwargs...)
end

function (self::ParamInterp)(fn::mystruct, args...; kwargs...)
    @show "cassette instance!!", args, kwargs
    @show fn(args...; kwargs...)
end

interp = ParamInterp()


test4() = inst(; y=3)
@test test4() == interp(test4)

So this may or may not be two bugs in a trench coat.

@aviatesk
Copy link
Member

function (self::ParamInterp)(fn::typeof(mystruct), args...; kwargs...)
    @show "cassette constructor!!", args, kwargs
    @show mystruct(; kwargs...)
end

is very bad definition since typeof(mystruct) === DataType.

If you define it correctly, i.e. fn::Type{mystruct}, the above test passes successfully.

@pepijndevos
Copy link
Author

pepijndevos commented May 11, 2023

Ok but my actual bug is still there

In CedarSim I'm doing the following, not even using the type.

using CassetteOverlay

struct ParamInterp{M, S} <: CassetteOverlay.AbstractBindingOverlay{M, S}
    parameters::Dict{Symbol, Any}
end

function ParamInterp(method_table::Union{Nothing, Core.MethodTable}=nothing)
    if method_table === nothing
        ParamInterp{nothing, nothing}(Dict())
    else
        ParamInterp{method_table.module, method_table.name}(Dict())
    end
end

function (self::ParamInterp)(lens::ParamLens, args...; kwargs...)
    @show lp = Dict(pairs(getfield(lens, :nt)))
    @show kp = Dict(kwargs)
    @show self.parameters
    merge!(self.parameters, lp, kp)
    lens(;kwargs...)
end


interp = ParamInterp()

inst = ParamLens((foo=1, bar=2))

test1() = inst()
test1()
interp(test1)

test2() = inst(; foo=3)
test2()
interp(test2)

Where ParamSim is

struct ParamSim{T, S, P} <: AbstractSim{T}
    circuit::T
    mode::Symbol # mode is used in initialisation, so it is required
    spec::S
    params::P

    # Always use `Core.Typeof()` for extra type-inferrence _magic_
    # This is necessary so that `Accessors.set()` maintains the `T == Type{Foo}`
    function ParamSim(circuit, mode, spec, params)
        return new{Core.Typeof(circuit),typeof(spec),typeof(params)}(circuit, mode, spec, params)
    end
end

In this case test1 hits the overlay, but test2 just completely bypasses it.

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

2 participants