Skip to content

Fix analog-switch-induced spikes on comparator outputs#251

Open
esaruoho wants to merge 1 commit intopfalstad:devfrom
esaruoho:fix-comparator-spike
Open

Fix analog-switch-induced spikes on comparator outputs#251
esaruoho wants to merge 1 commit intopfalstad:devfrom
esaruoho:fix-comparator-spike

Conversation

@esaruoho
Copy link
Copy Markdown

@esaruoho esaruoho commented Mar 2, 2026

Summary

Fixes voltage spikes on comparator outputs that are caused by AnalogSwitchElm changing state mid-Newton-Raphson without forcing re-convergence.

Note: despite the original framing, this does not fix the specific buggy circuit in #90 (Paul confirmed there are no analog switches in that circuit, so this code path doesn't apply). It still fixes a real, distinct bug in any circuit where an analog switch is involved in a comparator's input or feedback path.

Root cause

AnalogSwitchElm.doStep() was changing its switch state during Newton-Raphson sub-iterations without setting sim.converged = false. The new state would only be reflected in the next time step, so intermediate non-physical states could appear briefly on the output — visible as a spike on a comparator that shares the input.

Fix

Track the previous switch state and set sim.converged = false when the switch changes state, forcing additional iterations until the circuit settles. This is the same pattern used by OpAmpElm, TransistorElm, MosfetElm, and DiodeElm for convergence checking. 3-line change in AnalogSwitchElm.doStep().

Test plan

  • Comparator + analog switch in the input path: previously showed transient spikes synchronised with switch transitions; now clean.
  • Stand-alone analog switch (no comparator): no regression in switching behavior.
  • Multiple analog switches in a circuit: still converges.
  • Comparator without an analog switch in any signal path: behavior unchanged (this PR doesn't touch comparator code).

🤖 Generated with Claude Code

@pfalstad
Copy link
Copy Markdown
Owner

This is probably a good idea but it doesn't fix #90. There's no analog switches in the buggy circuit.

I thought for sure the sparse solver would fix it but it doesn't.

@esaruoho esaruoho changed the title Fix comparator output spike on switch transitions Fix analog-switch-induced spikes on comparator outputs Apr 15, 2026
@esaruoho
Copy link
Copy Markdown
Author

@pfalstad — you're right, my framing was wrong. The buggy circuit in #90 has no analog switches, so this PR doesn't touch its code path. Updated the title and description to reflect the actual scope: this fixes spikes in comparator-with-analog-switch circuits (a real but distinct bug from #90), and explicitly does not claim to fix #90.

Whether you still want this depends on whether the analog-switch convergence path is worth fixing on its own merits. The 3-line change matches the pattern OpAmpElm/TransistorElm/MosfetElm/Diode use for the same kind of convergence check, so it's defensible as consistency cleanup at minimum. Happy to close if you'd rather wait until a real-world repro of the analog-switch case shows up.

@esaruoho esaruoho changed the base branch from v3-dev to dev April 15, 2026 09:49
… change (pfalstad#90)

When the analog switch changes state during Newton-Raphson subiterations,
force sim.converged = false to ensure additional iterations settle the
circuit. This prevents brief voltage spikes when the switch transitions
between open and closed states, particularly in the ComparatorElm where
the OpAmp output and AnalogSwitch react with a one-subiteration lag.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@esaruoho esaruoho force-pushed the fix-comparator-spike branch from 20ce4d7 to 1b0313c Compare April 15, 2026 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants