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

Function ports is not type stable. #803

Open
samuelsonric opened this issue Jun 12, 2023 · 3 comments
Open

Function ports is not type stable. #803

samuelsonric opened this issue Jun 12, 2023 · 3 comments

Comments

@samuelsonric
Copy link

samuelsonric commented Jun 12, 2023

Consider the following two functions:

using Catlab.WiringDiagrams

wd = singleton_diagram(UntypedUWD, 1000000);

function collect_ports_1(wd)
    ps = Int[]
    for i in ports(wd)
        push!(ps, i)
    end
    ps
end

function collect_ports_2(wd)
    ps = Int[]
    for i in ports(wd)::UnitRange{Int}
        push!(ps, i)
    end
    ps
end

Then, on my machine,

collect_ports_1(wd)
@time collect_ports_1(wd);

outputs 0.051219 seconds (3.00 M allocations: 70.801 MiB).

On the other hand,

collect_ports_2(wd)
@time collect_ports_2(wd);

outputs 0.004928 seconds (17 allocations: 9.782 MiB).

@samuelsonric
Copy link
Author

samuelsonric commented Jun 12, 2023

The function junction has a similar problem:

using Catlab.ACSetInterface, Catlab.WiringDiagrams

wd = singleton_diagram(UntypedUWD, 1000000);

function collect_junctions_1(wd)
    js = Int[]
    for i in ports(wd)::UnitRange{Int}
        push!(js, junction(wd, i))
    end
    js
end

function collect_junctions_2(wd)
    js = Int[]
    _js = collect(subpart(wd, :junction))
    for i in ports(wd)::UnitRange{Int}
        push!(js, _js[i])
    end
    js
end

Then

collect_junctions_1(wd)
@time collect_junctions_1(wd);

outputs 0.206502 seconds (3.00 M allocations: 177.620 MiB).

On the other hand,

collect_junctions_2(wd)
@time collect_junctions_2(wd);

outputs 0.006355 seconds (19 allocations: 17.411 MiB)

@jpfairbanks
Copy link
Member

Thanks for the catch here. According to @code_warntype it is a problem in ACsets parts code.

julia> @code_warntype parts(wd, :Port)
MethodInstance for ACSets.ACSetInterface.parts(::UntypedUWD, ::Symbol)
  from parts(acs, type) @ ACSets.ACSetInterface ~/.julia/packages/ACSets/1pqJM/src/ACSetInterface.jl:36
Arguments
  #self#::Core.Const(ACSets.ACSetInterface.parts)
  acs::UntypedUWD
  type::Symbol
Body::Any
1nothing%2 = ACSets.ACSetInterface.nparts(acs, type)::Any%3 = (1:%2)::Any
└──      return %3

nparts has the same problem

julia> @code_warntype nparts(wd, :Port)
MethodInstance for ACSets.ACSetInterface.nparts(::UntypedUWD, ::Symbol)
  from nparts(acs::SimpleACSet, type::Symbol) @ ACSets.DenseACSets ~/.julia/packages/ACSets/1pqJM/src/DenseACSets.jl:341
Arguments
  #self#::Core.Const(ACSets.ACSetInterface.nparts)
  acs::UntypedUWD
  type::Symbol
Body::Any
1nothing%2 = Base.getproperty(acs, :parts)::ACSets.LVectors.LVector{(:Box, :Port, :OuterPort, :Junction), Int64}%3 = Base.getindex(%2, type)::Any
└──      return %3

@jpfairbanks
Copy link
Member

jpfairbanks commented Jun 14, 2023

Full MWE:

using Catlab.WiringDiagrams
using Catlab.CategoricalAlgebra

wd = singleton_diagram(UntypedUWD, 1000000);

@code_warntype nparts(wd, :Port)

I tested on Catlab 14 and 13 so this isn't a (recent) regression. with the ACSets refactor.

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