Skip to content

Add method to find connected components of SolidModels entities#190

Open
gpeairs wants to merge 4 commits intomainfrom
gp/connected-components
Open

Add method to find connected components of SolidModels entities#190
gpeairs wants to merge 4 commits intomainfrom
gp/connected-components

Conversation

@gpeairs
Copy link
Copy Markdown
Member

@gpeairs gpeairs commented Mar 27, 2026

Close #151 under the assumption that connected subgroups only need to be identified, not used in further postrender operations. Implements SolidModel.connected_components which takes physical group names or entity tags and a dimension and outputs a vector of the connected components in those groups or tags.

To create physical groups, you would run something like this after rendering:

metal_islands = SolidModels.connected_components(sm, "metal")
for i in eachindex(metal_islands)
    sm["metal_island_$i"] = metal_islands[i]
end

@gpeairs gpeairs requested a review from ybrightye March 27, 2026 14:43
@gpeairs gpeairs force-pushed the gp/connected-components branch from 6b63020 to fcca14d Compare March 27, 2026 14:45
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown

@ybrightye ybrightye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: return type mismatch in early exits of connected_components(dim, tags)

The two early returns in connected_components(dim::Int, tags::Vector{Int32}) return Vector{Vector{Int32}}, but the main code path returns Vector{Vector{Tuple{Int32, Int32}}} (dimtags):

isempty(tags) && return Vector{Int32}[]   # Vector{Vector{Int32}}
n == 1 && return [tags]                    # Vector{Vector{Int32}}

Should be:

isempty(tags) && return Vector{Tuple{Int32,Int32}}[]
n == 1 && return [[(Int32(dim), tags[1])]]

The "empty input" and "single entity" tests currently pass because they assert the buggy return type (result[1] == Int32[1]). They'd need updating too.

@gpeairs
Copy link
Copy Markdown
Member Author

gpeairs commented Apr 1, 2026

Bug: return type mismatch in early exits of connected_components(dim, tags)

Thanks, fixed.

@gpeairs gpeairs force-pushed the gp/connected-components branch from bf1bf40 to 1231b34 Compare April 1, 2026 18:34
Copy link
Copy Markdown

@ybrightye ybrightye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validated: flux loop workflow works end-to-end

Ran the following script against the PR branch (cherry-picked onto gp/channels-dev for compat with the ExamplePDK):

test/demo_connected_components.jl (click to expand)
using DeviceLayout, DeviceLayout.PreferredUnits, DeviceLayout.SolidModels
import DeviceLayout.SolidModels: connected_components
using DeviceLayout.SchematicDrivenLayout
using DeviceLayout.SchematicDrivenLayout.ExamplePDK
import DeviceLayout.SchematicDrivenLayout.ExamplePDK: LayerVocabulary

g = SchematicGraph("single-transmon")
q = ExamplePDK.Transmons.ExampleRectangleTransmon()
rr = ExamplePDK.ReadoutResonators.ExampleClawedMeanderReadout()
qubit_node = add_node!(g, q)
rres_node = fuse!(g, qubit_node, rr)
p_readout = Path(Point(0μm, 0μm); α0=π/2, name="p_ro", metadata=LayerVocabulary.METAL_NEGATIVE)
straight!(p_readout, 2mm, Paths.CPW(10μm, 6μm))
straight!(p_readout, 2mm, Paths.CPW(10μm, 6μm))
csport = CoordinateSystem(uniquename("port"), nm)
render!(csport, only_simulated(centered(Rectangle(10μm, 10μm))), LayerVocabulary.PORT)
attach!(p_readout, sref(csport), 10μm, i=1)
attach!(p_readout, sref(csport), 2mm - 10μm, i=2)
p_readout_node = add_node!(g, p_readout)
attach!(g, p_readout_node, rres_node => :feedline, 0mm, location=1)
floorplan = plan(g)
chip = offset(bounds(floorplan), 2mm)[1]
render!(floorplan.coordinate_system, chip, LayerVocabulary.SIMULATED_AREA)
render!(floorplan.coordinate_system, chip, LayerVocabulary.WRITEABLE_AREA)
render!(floorplan.coordinate_system, chip, LayerVocabulary.CHIP_AREA)
check!(floorplan)

sm = SolidModel("test", overwrite=true)
render!(sm, floorplan, ExamplePDK.singlechip_solidmodel_target(["port_1","port_2","lumped_element"]); strict=:no)

# --- Detect connected components ---
metal_comps = connected_components(sm, "metal")
active_comps = connected_components(sm, ["metal", "lumped_element", "port_1", "port_2"])

# --- Assign as named groups (flux loop workflow from issue #151) ---
for i in eachindex(metal_comps)
    sm["metal_island_$i"] = metal_comps[i]
end

@assert length(metal_comps) == 3  # ground, island, CPW trace
@assert length(active_comps) == 1  # all connected through ports
for i in 1:3
    @assert haskey(SolidModels.dimgroupdict(sm, 2), "metal_island_$i")
end

Output:

metal: 7 entities → 3 connected components
metal + lumped_element + ports: 1 connected component(s)

metal_island_1: 1 entities, grouptag=41
metal_island_2: 2 entities, grouptag=42
metal_island_3: 4 entities, grouptag=43

✓ All assertions passed

This validates the real use case from #151: automatically detecting disconnected regions of a physical group and assigning them as named subgroups for Palace simulation config. Previously this required hardcoding the number of subgroups (e.g., for i in 1:3 in our magnetostatic setup scripts).

Suggestion: consider adding this as a test alongside the existing metal_conn_comps assertions in test_examples.jl — specifically the sm["metal_island_$i"] = metal_comps[i] assignment + retrieval, since that's the workflow users actually need and it exercises the setindex! path with dimtags from connected_components.

Also noticed commit 1231b34 fixed the return type bug I flagged earlier — thanks!

@gpeairs
Copy link
Copy Markdown
Member Author

gpeairs commented Apr 2, 2026

Suggestion: consider adding this as a test alongside the existing metal_conn_comps assertions in test_examples.jl — specifically the sm["metal_island_$i"] = metal_comps[i] assignment + retrieval, since that's the workflow users actually need and it exercises the setindex! path with dimtags from connected_components.

Good call, added.

@gpeairs gpeairs force-pushed the gp/connected-components branch from 2517377 to 23c18df Compare April 2, 2026 08:59
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 this pull request may close these issues.

Postrender operation that splits a physical group into subgroups

2 participants