sim: correctness fixes — wired parameters, PushButton/Monoflop Reset, InputRef mirroring - #16
Open
discostu105 wants to merge 3 commits into
Open
sim: correctness fixes — wired parameters, PushButton/Monoflop Reset, InputRef mirroring#16discostu105 wants to merge 3 commits into
discostu105 wants to merge 3 commits into
Conversation
The parser signature for PushButton/PushButton2/PushButton2Sel only mapped InputTrigger and On, so a wired Reset landed at an input index the eval never read — a PushButton driven off via Reset stayed on forever. - parser: add Reset + InputDisable to the PushButton and PushButtonSel input signatures (synthesized when absent, so layouts stay stable) - interpreter: Reset dominates On and forces off; InputDisable gates the trigger edge (PushButton and PushButton2; assumed behavior, flagged) - JIT: same semantics in EvalStep::PushButton (usize::MAX sentinel for absent connectors) so interpreter/JIT equivalence holds Found while simulating the r50 UV-lamp weekday scenario (PushButton with On: weekday-Or, Reset: Not(weekday-Or)).
…d AQ - Monoflop: the parser signature lacked Reset, so a wired Reset was never read and a running pulse could not be aborted. Reset now clears the countdown and blocks retriggering while held (interpreter + JIT; assumed behavior, flagged). - InputRef: the impl forwarded I->Q and AI->AQ separately, but the Miniserver mirrors the one fed side onto BOTH outputs — the r50 corpus wires 'ref.AI <- mem.AQ' with consumers reading ref.Q. Q is now the digital view (non-zero -> 1) and AQ the analog value of whichever side is fed. Found while simulating the r50 alarm chain (Monoflop 'alarm_halten' with Reset: heimkommen_ref.Q, triggered through an AI-fed InputRef).
Loxone lets any parameter (Formula Input1-Input4, comparator Input2, timer durations) be driven by a wire instead of a fixed value. The graph refused such wires (add_wire required dir == Input, degrading them to warnings), and both evaluators read parameters straight from the local connector signal, so a wired parameter silently kept its Def= value. - graph: add_wire accepts any non-Output sink - engine: BlockEvalInfo resolves param_sources through wires like input_sources (feedback reads prev-tick; autodiff duals follow too) - compiler (JIT): the input_source map covers parameter connectors and every EvalStep param index resolves through it Found via lxir's pool example: a VirtualIn wired into a Formula's Input1 evaluated as 0 instead of the injected value.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three independent correctness fixes for
lox-sim, found while building lxir'slxir testcommand, which compiles a test DSL into SimSpec JSON and runs it throughlox sim run --jsonagainst real configs.1. Wires into parameter connectors are honored (
8d44ffc)Loxone lets any parameter be driven by a wire instead of a fixed value — Formula
Input1–Input4, comparatorInput2, timer durations. Previously:Graph::add_wirerequired the sink to beConnectorDir::Input, so wires intoParameterconnectors were refused and degraded to warnings at parse time;Def=value.Now the graph accepts any non-Output sink, the interpreter resolves
param_sourcesthrough wires exactly likeinput_sources(feedback wires read prev-tick; autodiff duals follow the same resolution), and the JIT compiler'sinput_sourcemap covers parameter connectors with everyEvalStepparam index resolved through it.Observed failure: a
VirtualInwired into a Formula'sInput1evaluated as 0 instead of the injected value.2. Monoflop Reset; InputRef mirrors its fed side to Q and AQ (
1fb350d)Monoflopnow honors itsResetinput (cancels the running pulse).InputRefmirrors whichever side feeds it to bothQandAQ, matching how downstream blocks consume refs in real configs.3. PushButton honors Reset and InputDisable (
735bc8d)PushButton/PushButton2previously ignored theirResetandInputDisableconnectors.All three are test-covered (
cargo test -p lox-sim: 498 passing). Reset semantics that could not be validated against a live Miniserver are marked with the existing "WARNING: Assumed behavior" comment convention.Happy to split this into three PRs if you prefer.