dsharlet / LiveSPICE

Real time SPICE simulation for audio signals
MIT License
416 stars 61 forks source link

System.ArgumentException Incorrect number of arguments supplied for call to method #165

Open mdouchement opened 1 year ago

mdouchement commented 1 year ago

Based on https://github.com/dsharlet/LiveSPICE/commit/a0f0a8e4e75e2c81cc78d70f38aa67df48f1f0d8 build. I'm getting an error when I simulate the following shema: image

Building circuit...
Build: 0 errors, 0 warnings
Building solution for h=2,6041666666666666E-06
Performing steady state analysis...
Performing transient analysis...
System solved, 2 solution sets for 31 unknowns.
Exception: System.ArgumentException: Incorrect number of arguments supplied for call to method 'Double lambda_method67(System.Runtime.CompilerServices.Closure, Double, Double)' (Parameter 'method')
   at System.Dynamic.Utils.ExpressionUtils.ValidateArgumentCount(MethodBase method, ExpressionType nodeKind, Int32 count, ParameterInfo[] pis)
   at System.Linq.Expressions.Expression.Call(Expression instance, MethodInfo method, Expression arg0, Expression arg1)
   at System.Linq.Expressions.Expression.Call(Expression instance, MethodInfo method, IEnumerable`1 arguments)
   at ComputerAlgebra.LinqCompiler.CompileExpression.VisitCall(Call C)
   at ComputerAlgebra.ExpressionVisitor`1.Visit(Expression E)
   at ComputerAlgebra.LinqCompiler.CompileExpression.Visit(Expression E)
   at ComputerAlgebra.LinqCompiler.CompileExpression.VisitProduct(Product M)
   at ComputerAlgebra.ExpressionVisitor`1.Visit(Expression E)

By removing C4 or C5 the circuit is stable.

Shape2.schx ```xml ```
dsharlet commented 1 year ago

This example is uncovering an interesting bug that I think is a failure of the computer algebra simplification (or perhaps some higher level simplification first). The problem can be reproduced with just two capacitors in parallel. If the computer algebra system doesn't recognize this is equivalent to just one capacitor, then we end up with a system of equations with two differential unknowns and only one differential equation. The solver then does a dumb thing and lets differential unknowns propagate all the way to the end of the process, and the compiler tries to actually generate a call to D, which isn't a real callable function.

Here's the funny part: if you rotate one of the two capacitors 180 degrees, swapping the anode and the cathode, the computer algebra system solves it just fine :)

With two parallel capacitors with flipped orientation:

System of 5 equations and 5 unknowns = { _v1[t], GND[t], i_1[t], VC5[t], VC2[t] }
  -GND[t] + _v1[t]==V1[t]
  GND[t]==0
  VC5[t]==-GND[t] + _v1[t]
  VC2[t]==GND[t] - _v1[t]
  D[VC2[t], t]*-1E-08 + D[VC5[t], t]*1E-08 + i_1[t]==0

With the same orientation:

System of 4 equations and 4 unknowns = { _v1[t], GND[t], i_1[t], VC5[t] }
  -GND[t] + _v1[t]==V1[t]
  GND[t]==0
  VC5[t]==-GND[t] + _v1[t]
  D[VC5[t], t]*2E-08 + i_1[t]==0
dsharlet commented 1 year ago

There's a likely lame workaround for this: capacitors could canonicalize the ordering of their anode/cathode by sorting the nodes by name...

dsharlet commented 1 year ago

Actually, I found a more real solution by making the computer algebra more robust. This was a nice bug to fix, I can't believe it hasn't been uncovered by anything until now! Thanks for the report and sorry about the delay in fixing it.

mdouchement commented 1 year ago

Thank you for the fix.

mdouchement commented 1 year ago

I still have the issue with Shape2.shx and https://github.com/dsharlet/LiveSPICE/releases/tag/v0.14

dsharlet commented 1 year ago

Sorry about that. I reduced this again to a simple case. Before, it was just two capacitors in parallel from the same node. That was easy to fix. This new reproducer is similar, except there are two capacitors in series in one of the parallel branches.

This case is tricky. The problem is that we really need to recognize that this is equivalent to a single capacitor. In the parallel case, this is easy enough, just making the computer algebra system a bit better. However, this case is problematic. Even if I get the CAS to understand this is one capacitor, it means that the node between the two series capacitors wouldn't actually exist. So if you probed it, it wouldn't work.

That by itself isn't so terrible a result. But it seems to indicate a bigger problem with how I'm thinking about the solver.

dsharlet commented 1 year ago

I've been working on this issue, it can be reduced to the following simple circuit: image

I think other failures, e.g. #170 are due to this same issue.

The problem is clear:

  D[VC1[t], t]*0.0001 - D[VC3[t], t]*0.0001==0
  D[VC1[t], t]*-0.0001 - D[VC2[t], t]*0.0001 + i_1[t]==0
  D[VC2[t], t]*0.0001 + D[VC3[t], t]*0.0001 - GND[t]*0.01 + _v2[t]*0.01==0

This is a singular system of equations. The solution is easy as a human: observing that VC1 = VC3, and substituting. How to do this in a general way is not so clear. I think in some of the other examples of this problem involve networks of components, not just a single component.