[codex] Add ngspice backend and passive parasitics#50
Conversation
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #50 +/- ##
==========================================
- Coverage 88.93% 87.87% -1.06%
==========================================
Files 74 77 +3
Lines 5712 6419 +707
Branches 1289 1542 +253
==========================================
+ Hits 5080 5641 +561
- Misses 632 778 +146
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4ae19b87f
ℹ️ 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".
| const prepared = await prepareInput(input, options); | ||
|
|
||
| const warnings: SimulationWarning[] = []; | ||
| const result: SimulationResult = { warnings }; |
There was a problem hiding this comment.
When simulator: 'ngspice-wasm' is selected, this path compiles the input but never applies the native validateCircuit check. For inputs with devices but no .op/.dc/.tran/.ac, runAnalyses() iterates over an empty analysis list and returns a successful { warnings: [] } result instead of the documented InvalidCircuitError, so invalid user netlists can appear to have simulated successfully.
Useful? React with 👍 / 👎.
| const model = desc.modelName ? this._models.get(desc.modelName) : undefined; | ||
| if (desc.modelName && !model) { | ||
| throw new Error(`Capacitor '${desc.name}' references unknown model '${desc.modelName}'`); |
There was a problem hiding this comment.
SPICE model names are case-insensitive, but this lookup uses the passive's model name verbatim while addModel stores the original spelling. A valid netlist such as .model cstd C(CAP=1n) followed by C1 1 0 CSTD now throws references unknown model; normalize model keys on insertion and lookup so passive model support matches SPICE casing rules.
Useful? React with 👍 / 👎.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const trimmed = line.trim(); | ||
| if (!trimmed || trimmed.toLowerCase() === '.end') continue; | ||
|
|
||
| const mosfetMatch = line.match(/^(M\S+)\s+(\S+)\s+(\S+)\s+(\S+)\s+(NMOD|PMOD|NMOS\S*|PMOS\S*)\s*(.*)$/i); |
There was a problem hiding this comment.
🟡 MOSFET regex in adaptNetlistForNgspice fails on indented netlist lines due to ^ anchor on untrimmed input
The MOSFET 3→4 terminal adaptation regex at packages/core/src/simulators/ngspice-wasm.ts:310 matches against line (which includes leading whitespace) instead of trimmed. The ^ anchor in /^(M\S+)\s+.../ requires the MOSFET name at position 0, so any indented MOSFET line (e.g., M1 drain gate source NMOD W=1u) won't match. The line passes through unmodified as a 3-terminal MOSFET, which ngspice cannot parse (it requires 4 terminals), causing the simulation to fail. The trimmed variable is already computed at line 307 but not used for the match.
| const mosfetMatch = line.match(/^(M\S+)\s+(\S+)\s+(\S+)\s+(\S+)\s+(NMOD|PMOD|NMOS\S*|PMOS\S*)\s*(.*)$/i); | |
| const mosfetMatch = trimmed.match(/^(M\S+)\s+(\S+)\s+(\S+)\s+(\S+)\s+(NMOD|PMOD|NMOS\S*|PMOS\S*)\s*(.*)$/i); |
Was this helpful? React with 👍 or 👎 to provide feedback.
| wf.width ?? Infinity, | ||
| wf.period ?? Infinity, |
There was a problem hiding this comment.
🟡 formatWaveform serializes PULSE width/period as literal "Infinity" — invalid SPICE syntax
When a PULSE source is created programmatically without explicit width or period, the defaults at packages/core/src/circuit.ts:109-110 are Infinity. The formatNumber(Infinity) function returns the string "Infinity" (since Number.isFinite(Infinity) is false, it falls through to String(Infinity) at line 75). This produces invalid SPICE output like PULSE(0 5 0 1e-12 1e-12 Infinity Infinity) from toNetlist(), which ngspice cannot parse. This primarily impacts the ngspice-wasm backend when a programmatic Circuit with an incomplete PULSE spec is passed as input.
Prompt for agents
The formatWaveform function in circuit.ts uses Infinity as the default for PULSE width and period (lines 109-110), and formatNumber (line 74-76) converts Infinity to the literal string "Infinity" which is not valid SPICE syntax.
To fix this, either:
1. Have formatNumber map Infinity to a very large number string (e.g. 1e99) that SPICE parsers accept, OR
2. Change the PULSE defaults from Infinity to a very large finite number, OR
3. Filter out Infinity values before serialization and omit trailing PULSE parameters that are at their defaults.
Option 1 is simplest: in formatNumber, add a check like if (value === Infinity) return '1e99' (or similar large number). This keeps the semantics close to the original Infinity intent while producing valid SPICE.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Adds a swappable core simulator backend API with an optional ngspice WASM adapter powered by
eecircuit-engine.Also completes passive capacitor and inductor modeling by resolving model/instance parameters and expanding parasitic equivalent networks for AC/transient analysis.
What changed
SimulationOptions.simulator, simulator adapter types, andcreateSimulator()routing.WasmNgspiceSimulatorwith lazyeecircuit-engineimport, programmaticCircuitserialization, include preprocessing,.op/.dc/.tran/.ac/.stepresult mapping, and stream expansion support.eecircuit-engine.Validation
pnpm --dir packages/core exec vitest run src/simulators/simulator-backends.test.ts— 9 passedpnpm --dir packages/core run lintpnpm --dir packages/core test— 427 passed, 1 skippedpnpm --dir packages/core run buildgit diff --checkNotes
Unrelated untracked local files were intentionally not included:
output.gif,packages/ui/.codex/.