ENH: Add Support for dict unpacking in csp.node - #730
Conversation
Signed-off-by: Aadya Chinubhai <aadyachinubhai@gmail.com>
AdamGlustein
left a comment
There was a problem hiding this comment.
Thanks for the contribution, this is great! Just some minor comments to address.
| proxy = outputs_by_name[k] | ||
| except KeyError: | ||
| raise KeyError(f"unrecognized output '{k}'") from None | ||
| proxy + v |
There was a problem hiding this comment.
Please add a comment here noting we override the ast.Add operator to output a value, elsewise this code looks like it's dead. You may also need to do a ruff noqa to tell ruff to ignore it
| _CSP_ENGINE_START_TIME_FUNC = "_engine_start_time" | ||
| _CSP_ENGINE_END_TIME_FUNC = "_engine_end_time" | ||
| _CSP_ENGINE_STATS_FUNC = "_csp_engine_stats" | ||
| _CSP_OUTPUT_KWARGS_FUNC = "_csp_output_kwargs" |
There was a problem hiding this comment.
Nit: you can remove _CSP_OUTPUT_KWARGS_FUNC from the dicts here and just call _csp_output_kwargs in the ast.Call node directly. These dictionaries are meant for functions defined in the C++ code.
| # A **expr unpack, resolved at runtime, can't statically verify | ||
| # which outputs it covers, so assume it may cover all of them. | ||
| self._returned_outputs.update(o.name for o in self._signature._outputs if o.name is not None) | ||
| nodes.append( |
There was a problem hiding this comment.
we can try to avoid the function call overhead and generator the for loop AST directly here instead
note you can use @node( debug_print=True ) to dump the generated readable code for debugging
There was a problem hiding this comment.
Yes, I'm working on it, I saw the dict creation per tick is actually more expensive than the jump to the function call, so hopefully minimizing that too, so the same dict can be reused with different runtime values.
There was a problem hiding this comment.
not sure i follow, where would there be an extra dict creation? ** would be replaced with a loop on the variable being expanded, no new dict being created. we should add an isinstance check to your point above ( #730 (comment) )
There was a problem hiding this comment.
After the inline change it would generate:
while True:
yield
if +#inp_0:
values = {'a': 1, 'b': 2}
for k, v in values.items():
{'a': #outp_0, 'b': #outp_1}[k] + vA dict display is a runtime instruction, Python constructs a fresh dict every time it evaluates { }, so this builds one per iteration. The keys never change though, so instead take the { } outside the while True, right after the first yield, by then PyNode init has already patched the real proxies into the frame locals, so the map captures them once and stays valid for the node's lifetime:
yield
#csp_outmap = {'a': #outp_0, 'b': #outp_1} # built once with proxies
...
while True:
yield
if +#inp_0:
values = {'a': 1, 'b': 2}
for #k, #v in values.items():
#csp_outmap[#k] + #vHoping I'm not missing something obvious here!
There was a problem hiding this comment.
Right I forgot that you need to lookup the out proxy dynamically but yes of course you would build the map once outside of the generator loop
Signed-off-by: Aadya Chinubhai <aadyachinubhai@gmail.com>
|
Works. (csp-dev) aadya-chinubhai@aadya-laptop:~/Desktop/projects/csp$ python
Python 3.13.14 | packaged by conda-forge | (main, Jun 12 2026, 09:50:25) [GCC 14.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import csp
... from csp import ts
...
... @csp.node(debug_print=True)
... def foo(x: ts[bool]) -> csp.Outputs(a=ts[int], b=ts[int]):
... if csp.ticked(x):
... values = {"a": 1, "b": 2}
... csp.output(**values)
...
def foo():
#node_p = 'node_p', 0
#inp_0 = 'input_proxy', 0
#outp_0 = 'output_proxy', 0
#outp_1 = 'output_proxy', 1
x = 'input_var', 0
yield
del x
#csp_outmap = {'a': #outp_0, 'b': #outp_1}
while True:
yield
if +#inp_0:
values = {'a': 1, 'b': 2}
#csp_vals = values
if not isinstance(#csp_vals, dict):
raise TypeError('csp.output argument after ** must be a dict')
for #csp_k, #csp_v in #csp_vals.items():
#csp_outmap[#csp_k] + #csp_v
>>> Note: This still doesn't cover the following cases:
Are we looking to add any other operations? I can certainly work on it. |
Reference Issue:
Closes #725
Dict Unpacking via
**now works as shown above instead of aCspParseError.This includes the following:
Single dict Unpacking:
Nested dicts:
Multiple dicts
How
Named outputs get resolved at parse time:
csp.output(a=1)rewrites to#outp_0 + 1. That can't work for**values, since the keys only exist once the node ticks.So the parser detects the unpack (
ast.keywordwitharg=None) and emits a call to a small runtime helper, passing a statically-built{name: proxy}map alongside the unpacked expression. The helper iterates the dict at runtime and ticks each output via the same proxy + value mechanism the generated code already uses. No C++ changes needed, sincePyOutputProxyalready exposes this throughnb_add.