-
Notifications
You must be signed in to change notification settings - Fork 3
sim: correctness fixes — wired parameters, PushButton/Monoflop Reset, InputRef mirroring #16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -137,6 +137,8 @@ pub enum EvalStep { | |
| Monoflop { | ||
| trigger: usize, | ||
| prev_trigger: usize, | ||
| /// Reset signal slot; `usize::MAX` when the block has no Reset wire. | ||
| reset: usize, | ||
| param_duration: usize, | ||
| output: usize, | ||
| state_idx: usize, | ||
|
|
@@ -217,6 +219,10 @@ pub enum EvalStep { | |
| trigger: usize, | ||
| prev_trigger: usize, | ||
| force_on: usize, | ||
| /// Reset/InputDisable signal slots; `usize::MAX` when the block has | ||
| /// no such connector (PushButtonSel layouts vary). | ||
| reset: usize, | ||
| disable: usize, | ||
| /// outputs: [Q, Qoff, Qon] | ||
| outputs: [usize; 3], | ||
| state_idx: usize, | ||
|
|
@@ -384,11 +390,13 @@ impl CompiledGraph { | |
| // For feedback wires, we'll read from prev_signals instead. | ||
| let feedback_wires = &topo.feedback_wires; | ||
|
|
||
| // Build input source map: for each input connector, where does its value come from? | ||
| // Build input source map: for each input or parameter connector, | ||
| // where does its value come from? (Parameters can be wire-driven | ||
| // too, e.g. Formula Input1-Input4.) | ||
| let mut input_source: Vec<(usize, bool)> = vec![(0, false); n_conn]; | ||
| for (cid, src) in input_source.iter_mut().enumerate() { | ||
| let is_input = graph.connector(cid).dir == ConnectorDir::Input; | ||
| if is_input { | ||
| let is_sink = graph.connector(cid).dir != ConnectorDir::Output; | ||
| if is_sink { | ||
| match graph.input_source_of(cid) { | ||
| Some(from) => { | ||
| let is_fb = feedback_wires.contains(&(from, cid)); | ||
|
|
@@ -420,7 +428,9 @@ impl CompiledGraph { | |
| let prev_inputs: Vec<usize> = resolved_inputs.clone(); | ||
|
|
||
| let outputs = &info.outputs; | ||
| let params = &info.params; | ||
| // Wired parameters read their driving output's signal; unwired | ||
| // ones read their own connector (holding the Def= value). | ||
| let params: Vec<usize> = info.params.iter().map(|&cid| input_source[cid].0).collect(); | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wired Formula parameters are still ignored by compiled execution. Resolving parameter source indices here works for compiled block types such as Gain, but Formula has no compiled match arm and falls through to |
||
|
|
||
| let step = match block_type { | ||
| "And" => EvalStep::And { | ||
|
|
@@ -520,6 +530,7 @@ impl CompiledGraph { | |
| EvalStep::Monoflop { | ||
| trigger: resolved_inputs.first().copied().unwrap_or(0), | ||
| prev_trigger: prev_inputs.first().copied().unwrap_or(0), | ||
| reset: resolved_inputs.get(1).copied().unwrap_or(usize::MAX), | ||
| param_duration: params.first().copied().unwrap_or(0), | ||
| output: outputs[0], | ||
| state_idx: si, | ||
|
|
@@ -662,6 +673,8 @@ impl CompiledGraph { | |
| trigger: resolved_inputs.first().copied().unwrap_or(0), | ||
| prev_trigger: prev_inputs.first().copied().unwrap_or(0), | ||
| force_on: resolved_inputs.get(1).copied().unwrap_or(0), | ||
| reset: resolved_inputs.get(2).copied().unwrap_or(usize::MAX), | ||
| disable: resolved_inputs.get(3).copied().unwrap_or(usize::MAX), | ||
| outputs: [ | ||
| *outputs.first().unwrap_or(&0), | ||
| *outputs.get(1).unwrap_or(&0), | ||
|
|
@@ -969,18 +982,22 @@ impl CompiledGraph { | |
| EvalStep::Monoflop { | ||
| trigger, | ||
| prev_trigger, | ||
| reset, | ||
| param_duration, | ||
| output, | ||
| state_idx, | ||
| } => { | ||
| let trig = self.signals[*trigger]; | ||
| let prev_trig = self.prev_signals[*prev_trigger]; | ||
| let rst = self.signals.get(*reset).copied().unwrap_or(0.0); | ||
| let duration = self.signals[*param_duration].max(0.0); | ||
| let out = *output; | ||
| let si = *state_idx; | ||
|
|
||
| if let BlockState::Timer { remaining, .. } = &mut self.state[si] { | ||
| if prev_trig < 0.5 && trig >= 0.5 { | ||
| if rst >= 0.5 { | ||
| *remaining = 0.0; | ||
| } else if prev_trig < 0.5 && trig >= 0.5 { | ||
| *remaining = duration.max(dt); | ||
| } | ||
| let q = *remaining > 0.0; | ||
|
|
@@ -1315,20 +1332,26 @@ impl CompiledGraph { | |
| trigger, | ||
| prev_trigger, | ||
| force_on, | ||
| reset, | ||
| disable, | ||
| outputs, | ||
| state_idx, | ||
| } => { | ||
| let trig = self.signals[*trigger]; | ||
| let prev_trig = self.prev_signals[*prev_trigger]; | ||
| let force = self.signals[*force_on]; | ||
| let rst = self.signals.get(*reset).copied().unwrap_or(0.0); | ||
| let dis = self.signals.get(*disable).copied().unwrap_or(0.0); | ||
| let outs = *outputs; | ||
| let si = *state_idx; | ||
|
|
||
| if let BlockState::PushButton { is_on } = &mut self.state[si] { | ||
| let previous = *is_on; | ||
| if force >= 0.5 { | ||
| if rst >= 0.5 { | ||
| *is_on = false; | ||
| } else if force >= 0.5 { | ||
| *is_on = true; | ||
| } else if prev_trig < 0.5 && trig >= 0.5 { | ||
| } else if dis < 0.5 && prev_trig < 0.5 && trig >= 0.5 { | ||
| *is_on = !*is_on; | ||
| } | ||
| let qon = !previous && *is_on; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Compiled execution does not mirror
InputRef.CompiledGraph::from_graphhas noInputRefarm, so this block falls through toEvalStep::Copy, which only copies the first input to the first output. In a Docker repro withAI=42,SimEnginereturnsQ=1, AQ=42, butCompiledGraphreturnsQ=0, AQ=0. Please add an InputRef-specific compiled step covering both inputs and outputs, plus an enabled equivalence case using the realI,AI,Q,AQlayout.