Fix ParameterSet round trip and @variant keyword handling

[parrangoiz]

Fixes #335, #336, #337

Fixes to how ParameterSets feed components and variants.

• create_component(T, ps, address) drops nested namespaces #335: create_component(T, ps, address) and set_parameters(c, ps, address) now read a nested namespace below address back as the parameter it names, so what extract_parameter_set writes can be read back:
  • A NamedTuple parameter is merged recursively into the default or template value, so the namespace only needs the fields that change.
  • A Dict parameter is replaced by the namespace's entries, with keys converted to the parameter's key type.
  • A namespace naming a parameter of any other type throws an ArgumentError. Previously it silently kept the default.
  • Namespaces that don't name a parameter, such as composite subcomponents, are still ignored. Leaves inside nested namespaces are recorded in ps.accessed.
• @variant / @composite_variant constructors accept unknown keyword arguments #336: @variant and @composite_variant constructors throw the same MethodError as the base @compdef constructor for keywords that aren't parameters of the variant. Previously a typo, including one in a ParameterSet namespace, was silently stored as a new parameter.
• A @composite_variant built from a ParameterSet never sees it in _build_subcomponents #337: A @composite_variant built with create_component(T, ps, address) now carries ps in its graph, so the base composite's _build_subcomponents sees it through parameter_set(cc._graph). The variant constructor accepts _graph, _schematic and _hooks like a @compdef constructor instead of storing them as parameters. base_variant attaches the same ParameterSet to the base composite's graph.
• Separate fixes:
  • create_component(T, ps, address) with an empty address, or one that resolves to a leaf value, now throws an ArgumentError with an actionable message, as set_parameters(c, ps, address) already did. Previously it threw a generic MethodError, or for composites failed while building a ParameterKeyError.
  • set_parameters on a composite keeps the ParameterSet attached to its graph. Previously the rebuilt instance's _build_subcomponents no longer saw it.

Docstrings, the ParameterSet tutorial and the CHANGELOG are updated.

Also cleaned up the docstrings of create_component, set_parameters (including the callable form c(name, params; kwargs...)) and extract_parameter_set to make them user friendly: they now describe only the outward-facing behavior, with examples.

Testing

New tests in test/test_parameter_set.jl and test/test_schematicdriven.jl; no existing tests changed.

[codecov]

Codecov Report

❌ Patch coverage is 98.55072% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/schematics/components/components.jl	98.03%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[ad-cqc]

This still leads to missing parameters set as it's never passed from the composite component parent:

 @compdef struct MWEComposite <: CompositeComponent
      name = "t"
  end

  function SchematicDrivenLayout._build_subcomponents(c::MWEComposite)
      ps = parameter_set(c._graph)
      ps === nothing && error("_build_subcomponents: no ParameterSet on _graph")
      return (create_component(ExampleRectangleIsland, ps, "components.t.island"),)
  end

  ps1 = ParameterSet(); ps1.components.t.island.cap_width = 42μm
  ps2 = ParameterSet(); ps2.components.t.island.cap_width = 99μm

  # (a) stale set
  tr  = create_component(MWEComposite, ps1, "components.t")
  tr2 = set_parameters(tr, ps2, "components.t")
  parameter_set(tr2._graph) === ps2                      # false — still ps1

  # (b) no set at all
  tr3 = set_parameters(MWEComposite(), ps2, "components.t")
  parameter_set(tr3._graph) === ps2                      # false — nothing

[ad-cqc]

A typo inside a nested NamedTuple namespace is accepted silently. For a component with style = (; trace=10μm, gap=6μm):

ps = ParameterSet()
ps.components.line.style.trce = 3μm   # meant `trace`

create_component(MWELine, ps, "components.line").style
# (trace = 10 μm, gap = 6 μm, trce = 3 μm)
set_parameters(MWELine(), ps, "components.line").style
# same
"components.line.style.trce" in ps.accessed   # true

Nothing errors, trace keeps its default, and the typo is recorded in ps.accessed, so an unused-parameter check won't catch it either. A top-level typo (ps.components.line.lenght) already throws an ArgumentError, which is what I'd expect here too.

I would suggest to pass the default or template value down from _namedtuple_namespaces (it already has base[s]), and throw an ArgumentError naming the full path and the valid fields when a key isn't in it, at every nesting level.

[ad-cqc]

A namespace for a required (no-default) NamedTuple or Dict parameter gets dropped here, and the constructor then fails with an error that doesn't mention the namespace:

@compdef struct MWEReq <: Component
    name = "req"
    style::NamedTuple
end

ps = ParameterSet()
ps.components.req.style.trace = 5μm
create_component(MWEReq, ps, "components.req")
# UndefKeywordError: keyword argument `style` not assigned

It fails the same way for labels::Dict{String, Any}, and passing the value directly (MWEReq(; style=(; trace=5μm))) works.

The cause is that default_parameters(T) only has parameters with defaults, so haskey(base, s) in _namedtuple_namespaces (line 195) is false for style. The namespace is then skipped like any other namespace that isn't a parameter.

So for names in parameter_names(T) that aren't in default_parameters(T), you should check if component fields have corresponding branch in the NamedTuple or Dict.

[ad-cqc]

This line assumes every AbstractCompositeComponent has a _graph field, but BasicCompositeComponent stores its graph in graph. The lookup falls through getproperty to the inner MetaGraph, so on this branch:

g = SchematicGraph("g")
add_node!(g, ExampleRectangleIsland())
bcc = BasicCompositeComponent(g)
set_parameters(bcc)
# FieldError: type MetaGraphs.MetaGraph has no field `_graph`, available fields: `graph`, `vprops`, ...

I think we need graph(c) defined for every composite, so ps = parameter_set(graph(c)) avoids the field lookup. The catch is that for @compdef composites, graph(c) builds the subcomponents, so it isn't a drop-in change. The simpler option is to guard with hasfield(typeof(c), :_graph) and use the generic method otherwise.

[ad-cqc]

The round trip breaks when a Dict is nested inside a NamedTuple parameter: extract_parameter_set writes it out, but it comes back as a NamedTuple.

@compdef struct MWEStyled <: Component
    name = "line"
    style = (; trace=10μm, extra=Dict{String, Any}("k" => 1))
end

g = SchematicGraph("g")
add_node!(g, MWEStyled())
ps = extract_parameter_set(g)
c = create_component(MWEStyled, ps, "components.line")

typeof(MWEStyled().style.extra)  # Dict{String, Any}
typeof(c.style.extra)            # @NamedTuple{k::Int64}

_extracted_parameter_value writes NamedTuples and Dicts the same way, as a nested Dict{String, Any}, so the namespace alone can't tell them apart. This branch then turns every nested Dict into a NamedTuple.

We should pass down the parameter's default or template value right type at each level. Recurse here when the matching field is a NamedTuple, and call _namespace_to_dict! when it's an AbstractDict.

[ad-cqc]

I think ArgumentError is better suited here then MethodError which requires non-public get_world_counter.

throw(ArgumentError("unknown keyword(s) for $T; valid parameters are ..."))