Repository navigation
fix: keep bit widths consistent across connections and components - #78
Merged
Merged
Conversation
The store counted its revision on a hand-written list of events, and `useCanSimulate` subscribed to a similar but slightly different list. Expose that list once as `subscribeToCircuitChanges`, so that anything derived from the whole circuit listens to exactly the changes the revision counts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Connections whose two ends came to disagree on their bit width were only dropped on a config change, and only inside the component of the node. They survived when a split pin of an aggregate node was resized, or when a change inside a component changed the width of one of its pins, so the simulation silently fed values of the wrong width onward. Dropping them everywhere was tried before and rejected: re-checking every connection after each change grew with the cube of the circuit size, and it dropped valid connections. The inference assumed that every pin of a user-defined node whose width is not fixed shares one width, so two independent paths through a component were forced to the same width. Group the pins instead. Within a component, pins share a bit width group when a connection ties them, when they are inferred pins of one intrinsic node, or when they are pins of one user-defined node whose implementations share a group inside it. A group takes the width its pins fix, and is in conflict when they fix more than one. The groups of a component are computed in one pass and cached until the circuit changes. A connection in a conflicting group is kept, since there is no undo and a later change can resolve the conflict. A new connection that would cause a conflict is still refused. The netlist gives each group of a component instance the width the pins of the node using it hand over, instead of a single width for the whole instance, and refuses to simulate a component with a conflict. In a chain of a constant and 100 not nodes, adding a node now takes 0.4 ms and deleting five 1.2 ms, against 207 ms and 1.4 s for the rejected re-check of every connection. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The netlist refuses a circuit whose connected pins disagree on their bit width, but a value can still come out wrong: an intrinsic node can compute a value of the wrong width, and the caller can supply an input value that does not fit its pin. Either used to flow on silently. Check the width of every value an instance reads or writes, and name the pin in the error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Connections whose tied pins fix different bit widths are now kept rather than dropped, so the editor has to show them. Draw them red and dashed, and mention the conflict among the reasons the simulation can fail. The conflict can be caused or resolved by a change anywhere in the circuit, such as inside a component used by a node at one end, so the renderer re-checks it on every circuit change. The check reads a cached analysis, so that costs a lookup per connection. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ponent "Create a new component" copies the selected connections into the new component and then removes the originals. The copies went through the bit width check, so a connection in conflict, which is now kept instead of dropped, was refused as a copy while its original was still removed. Let `register` keep a connection in conflict when asked to, and use that for the copies. Also document that a connection is only checked against the component it is drawn in, so one inside a component can still cause a conflict in the components using it, like a config change inside it can. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Collect the inputs of an instance outside the `try` that prefixes evaluation errors with the instance, so a misfit input no longer names the instance twice. - Update the description of `defaultBitWidth` of `simulateComponent` to match the netlist. - Stop exporting `requireManualBitWidth`, which only its module uses. - Note that `importJson` must run before `mount`, since some stores add the objects without announcing them and so without clearing the bit widths derived from them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- A component used twice at different widths, two levels deep. - An input value as wide as the pins tied to its input pin. - Bit width queries on every change event while objects are removed. - A connection inside a component causing a conflict where it is used. - A connection in conflict copied as it is only when asked to. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A connection touching a group in conflict was always refused, so a conflicting net could not be extended or probed until it was fixed. Allow a connection whose other side fixes no width. Those pins take whatever width the group settles on, so fixing the original conflict fixes them too. A pin fixing a width of its own is still refused, even one the group already has, since it would stay in conflict if the conflict were fixed the other way. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Deploying create-cpu-c8345196-9409-4cce-afc4-389f413ad8ee with
|
| Latest commit: |
6e55865
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://7282a97e.create-cpu-c8345196-9409-4cce-afc4-389f413ad8ee.pages.dev |
| Branch Preview URL: | https://fix-bit-width-consistency.create-cpu-c8345196-9409-4cce-afc4-389f413ad8ee.pages.dev |
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
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.
Summary
On main, a connection whose two ends came to disagree on their bit width could survive, and the simulation then silently passed values of the wrong width. Two cases were missed:
didUpdateConfighandler inconnection.tsonly looked inside the same component.An earlier attempt re-checked every connection after each change and dropped the inconsistent ones. It was abandoned for two reasons:
Design
Bit width groups (
src/store/bitWidth.ts). Within a component, pins share a group whenA,BandOutofAnd), orPins of independent paths through a component therefore stay apart. Some pins fix their own width: configurable pins, calculated pins, and pins of user-defined nodes whose implementation group is fixed. A group takes the width its pins fix, and is in conflict when they fix more than one. The groups of a component are computed in one pass with union-find and cached until the circuit changes.
Conflicts are kept, not dropped. There is no undo, and a later change can resolve a conflict, so a connection in a conflicting group stays and is drawn red and dashed.
Simulation.
SimulationBitWidthConflictError.Behavior changes
In → Outplus an independentTrue → Not → B, and is used with an 8-bit input.Bis now 1 bit; it used to be 8.Performance
Measured on a chain of a
Constand 100Notnodes, as in the review of the abandoned attempt:Testing
npx vitest run: 56 tests pass. New tests insrc/store/bitWidth.test.tscover:npx tsc -p .andnpx biome check srcpass.🤖 Generated with Claude Code