feat: multi-output task support#635
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd0d183fa5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let msg_type = | ||
| graph.get_connection_msg_type(pid, id).unwrap_or_else(|| { | ||
| panic!("Missing connection type from node {pid} to {id}") | ||
| }); |
There was a problem hiding this comment.
Use per-edge msg type for multi-output inputs
This lookup uses get_connection_msg_type(pid, id), which in CuGraph is backed by find_edge and only returns the first edge between the two nodes. If a multi-output task connects multiple msg types to the same downstream task (two cnx entries with the same src/dst but different msg), every input from that parent will be tagged with the first msg type and src_port. That makes the generated input tuple pull the wrong port (or duplicate the first type), so the build fails or the runtime misroutes the second connection. This needs to be resolved per edge, not per node pair.
Useful? React with 👍 / 👎.
| - Port ordering follows the first appearance order in `cnx` for that source task. | ||
| - Fan-out with the same `msg` type shares the same output port. | ||
|
|
||
| Limitation: A task cannot expose two distinct output ports with the same payload type. |
There was a problem hiding this comment.
Giving it some thoughts but I think it might be reasonable as a quirk. The alternative would be named outputs like the bridges but this makes this super complicated and wonky on the backward compat.
just dag still works :)
|
| } | ||
|
|
||
| fn process(&mut self, _clock: &RobotClock, output: &mut Self::Output<'_>) -> CuResult<()> { | ||
| output.0.set_payload(42); |
There was a problem hiding this comment.
maybe add a TOV on one of the output as they are independent
| @@ -0,0 +1,11 @@ | |||
| ( | |||
There was a problem hiding this comment.
Can we do like the bridge testing here and add a bunch of missions with some variation in the routing like here https://github.com/copper-project/copper-rs/blob/master/examples/cu_bridge_test/copperconfig.ron because there are a lot of corner cases:
what if one task gets both inputs? if an output is left open? Is a loopback error is correctly spotted?
There was a problem hiding this comment.
- Both inputs: covered by the both_inputs mission in
examples/cu_multi_output/copperconfig.ronand
tasks::BothSinkinexamples/cu_multi_output/src/main.rs. - Output left open: covered by the open_output mission in
examples/cu_multi_output/copperconfig.ronusing
tasks::DropBoolSinkinexamples/cu_multi_output/src/main.rsto intentionally drop one output. - Loopback error: validated via a dedicated config+test (
core/cu29_runtime/tests/loopback_config.ron,core/ cu29_runtime/tests/loopback.rs) and explicit detection incore/cu29_runtime/src/curuntime.rs. The example
config no longer includes a self-loop because it breaks compile-time planning.
…an receive multiple outputs from the same source without type-order mismatches; adds a TOV on the first output, includes the extra sinks/relay task, and updates LoopbackTask to a single-input pass-through; adds the missions/routing variants and removes the self-loop to keep the graph acyclic
Summary
Implemented multi-output task support end-to-end (runtime plan, codegen, monitoring topology, sim support) with deterministic port ordering and backward compatibility
Related Issue: #383
Detail Changes
core/cu29_runtime/src/cutask.rs,core/cu29_runtime/src/curuntime.rs).core/cu29_runtime/src/monitoring.rs,core/cu29_runtime/src/config.rs).core/cu29_derive/src/lib.rs).examples/cu_multi_output,Cargo.toml,README.md).