search

ra4king / CircuitSim

Basic Circuit Simulator
https://ra4king.github.io/CircuitSim
BSD 3-Clause "New" or "Revised" License
76 stars 27 forks source link

Bug: "short circuit detected" in v1.8.3 when there is no short circuit #73

Closed sameer-s closed 2 years ago

sameer-s commented 2 years ago

See the NOR gate circuit below: image

When A is toggled from 0 to 1, this (sometimes) causes a short circuit exception, even though there is no short circuit. This is somewhat important for 2110, since it causes our autograder to fail. I'm not sure if this is related to the transistor changes or the short circuit handling changes yet, but I'll see if I can identify more details tomorrow.

The circuit file can be found here: https://gist.github.com/sameer-s/58cf91ed621ca47b9e065acebb9cc06c

sameer-s commented 2 years ago

Some interesting notes on reproducibility: this error does not occur every time the file is loaded (in fact, it happens less often than not). However, if the error does show up, then it happens every time you toggle from 1 to 0. I'm still working on trying to find a root cause.

ausbin commented 2 years ago

Nice, I can reproduce this on master with your test circuit. I can't reproduce with only #72 checked out, and also I can reproduce this with the old transistor component too (Circuit file: Test2.sim.gz).

Your fix #74 seems to fix it for me locally for both old transistors (my .circ file above) and new transistors (your .circ file)

ra4king commented 2 years ago

I can reproduce as well, thanks for your report! This is due to my removal of the short-circuit retrying logic in commit fb2ae68454b47e3a1bc5b4dc7c46db6b6c1ebafa. Retrying the short-circuits is wasteful since the links should already be in a steady state when all links finish propagating, but that was a great catch that stepAll() would save the exception and throw it regardless of the updated states. I have reworked the code to track the short-circuited links and check whether they're still in a short-circuited state when the stepping is complete. You can see this in commit cd610622fde6c5d7635e37fc98779574c632c3e2.

I'll close this bug once you also confirm it's fixed on your end with my new commit.

sameer-s commented 2 years ago

Working for me! Thanks for the quick response as well, it seems like a better fix for sure and we should be able to get everything in before our first project releases.

ra4king commented 2 years ago

Sounds good, bumping to 1.8.4 and releasing. Please use the 1.8.4 revision.

ra4king commented 2 years ago

https://github.com/ra4king/CircuitSim/releases/tag/v1.8.4