Added ParameterSet for data-driven design parameterization#204
Added ParameterSet for data-driven design parameterization#204ad-cqc wants to merge 4 commits intoaws-cqc:mainfrom
Conversation
Introduce `ParameterSet`, a mutable nested dictionary wrapper that provides dot-access syntax for reading and writing design parameters. Supports `global` and `components` namespaces, programmatic construction, and optional YAML serialization via a package extension. Key changes: - Add `ParameterSet` type with `resolve` and `leaf_params` utilities - Add `ParameterSetYAMLExt` weak dep extension for YAML load/save - Integrate `parameter_set` into SchematicDrivenLayout and `@component` - Add comprehensive API reference documentation - Register YAML as optional dependency in Project.toml
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Replace Pair syntax with dot-access for setting initial parameters, rewrite composite component section to show cleaner pattern where subcomponent parameters live in the ParameterSet rather than the composite struct, remove fallback dual-path (if/else isnothing) code, and update YAML examples to match the simplified structure.
Replace silent auto-vivification of missing keys with a MissingNamespace sentinel that supports chained dot-writes but throws a descriptive ParameterKeyError when used as a value. Add tree-style pretty printing, a validate() function to detect unaccessed parameters, and comprehensive tests for the new error handling and display behavior. Update docs to use explicit parameter set paths for shared parameter forwarding.
Support parsing unit-suffixed values (e.g., "150μm") in YAML files loaded into ParameterSet. Values with recognized Unitful suffixes are automatically converted to proper quantities. Update documentation examples to use unitful parameters and add comprehensive tests for YAML round-tripping with units.
gpeairs
left a comment
gpeairs
left a comment
There was a problem hiding this comment.
Thanks, this is much needed. My major question is about precedence of the ParameterSet versus _build_subcomponent overrides -- right now it's ambiguous and up to the designer to follow the feature's intent. I have some ideas below about enforcing one way or the other, but I'm still undecided on what's best.
|
|
||
| function Base.getproperty(d::MissingNamespace, s::Symbol) | ||
| s in (:parent, :key, :accessed) && return getfield(d, s) | ||
| return MissingNamespace(d, String(s), getfield(d, :accessed)) |
There was a problem hiding this comment.
Can you explain the problem the MissingNamespace type solves? Why not throw an error right here? Edit: Ok, I think I understand the auto-vivifying on write. I'd worry about errors after reads that don't show an informative ParameterKeyError.
There was a problem hiding this comment.
The original idea is declarative definition of parameters. Parameter is allowed to be set at any namespace level event if it doesn't exist. But, reading from non-existing namespace should be prohibited. For this I use assignment operator on a parameter set namespace, e.g. ps.a.b = 10 where ps is a parameter space object. So,
ps.a.b = 10creates{'a': {'b': 10}}ps.a.breturns10as it was defined by an above definitionps.a.cis an error becausecwas never defined
But, because ps.a.b = 10 is translated as setproperty!(getproperty(ps, :a), :b, 1) where a final namespace key, :b, is a second parameter of setproperty!, we need a lazily auto-vivified wrapper around PS to facilitate namespace construction.
So, I cannot throw any missing key error in Base.getproperty(d::MissingNamespace, s::Symbol) because it generates me a first parameter of setproperty!, and by the time I'm inside setproperty! I do not care if namespace doesn't exist, it's created by the assignment operator.
ParameterKeyError is a minor patch for auxiliary functions missing namespace handling, but getproperty(d::MissingNamespace) will always return MissingNamespace object, and that's how user would be able to check existence of a namespace: ps.x is MissingNamespace.
| ExampleSimpleJunction, ps, "components.transmon.junction" | ||
| ) | ||
| # Forward shared parameter under the subcomponent's own name | ||
| junction = set_parameters(junction; h_ground_island=ps.components.transmon.junction_gap) |
There was a problem hiding this comment.
What would you think of applying the parameter set automatically in add_node!, since it's now present in the graph? Then we wouldn't require any changes to the component definition, and we could also avoid ambiguity about precedence.
If we only do that, then the designer can still forward whatever they like in _build_subcomponents, but anything present in the ParameterSet would be overridden. So we would lose any designer-intended consistency if the PS overrides something that’s supposed to be dependent.
Alternatively, if we want to enforce consistency, we can rely on the designer following the templates pattern to allow setting arbitrary subcomponent parameters. Then to set them via the PS, we just say subcomponent addresses in the PS are just aliases for the subcomponent templates. In other words, components.transmon.island really overrides components.transmon.templates.island. PS overrides the templates, then _build_subcomponents always overrides the PS to enforce CompositeComponent invariants. I'm not committed to that path, but it seems preferable to relying on the designer to correctly apply the ParameterSet in _build_subcomponents.
In that case, we wouldn't even need the CompositeComponent to hold onto the PS after creation.
| ps = parameter_set(tr._graph) | ||
|
|
||
| @component island = create_component( | ||
| ExampleRectangleIsland, ps, "components.transmon.island" |
There was a problem hiding this comment.
Would this be ”components.$(name(tr)).island" in general?
| - `data::Dict{String, Any}`: nested parameter dictionary | ||
| - `accessed::Set{String}`: tracks which parameter paths were consumed (for auditing) | ||
| """ | ||
| mutable struct ParameterSet |
There was a problem hiding this comment.
Does the ParameterSet itself have to be mutable? The container contents can be updated either way.
There was a problem hiding this comment.
No, immutability is ok
|
|
||
| const _REQUIRED_NAMESPACES = ("global", "components") | ||
|
|
||
| function _ensure_required_namespaces!(data::Dict{String, Any}) |
There was a problem hiding this comment.
I think it would be more idiomatic to guarantee these with an inner constructor, unless I’ve missed a reason not to.
| # Track leaf access | ||
| push!(getfield(ps, :accessed), key) |
There was a problem hiding this comment.
This just tracks the leaf parameter symbol -- do we need to track the whole path like in create_component? (But then inheriting ps.accessed has path prefixes no longer present.)
There was a problem hiding this comment.
Since this is only relevant for working with components, it should go in the src/schematics folder and be included in SchematicDrivenLayout.jl.
| ExampleRectangleIsland, ps, "components.transmon.island" | ||
| ) | ||
| # Forward shared parameter from parameter set to island | ||
| island = set_parameters(island; junction_gap=ps.components.transmon.junction_gap) |
There was a problem hiding this comment.
Here it's up to the component definition which of the ParameterSet or derived parameter takes precedence. The PS won't reflect any changes applied by set_parameters. My understanding is that's intentional (PS is just input parameters, final parameters can be extracted later and compared if necessary), but just want to double check.
2 participants
Introduce
ParameterSet, a mutable nested dictionary wrapper that provides dot-access syntax for reading and writing design parameters. Supportsglobalandcomponentsnamespaces, programmatic construction, and optional YAML serialization via a package extension.Key changes:
ParameterSettype withresolveandleaf_paramsutilitiesParameterSetYAMLExtweak dep extension for YAML load/saveparameter_setinto SchematicDrivenLayout and@component