Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable dynamic parameters #94

Open
dsharlet opened this issue Nov 4, 2020 · 14 comments · May be fixed by #238
Open

Enable dynamic parameters #94

dsharlet opened this issue Nov 4, 2020 · 14 comments · May be fixed by #238

Comments

@dsharlet
Copy link
Owner

dsharlet commented Nov 4, 2020

Currently, when you change a potentiometer or other circuit control, the simulation has to be resolved, which takes a while, and loses the state of the circuit.

A while back, this worked differently: the potentiometers and other parameters were actual parameters of the simulation function, and could be adjusted smoothly and dynamically. This was a much better user experience and would make circuits like Wah pedals a lot more useful and interesting.

However, this approach really struggled, because the non-constant expressions would get gigantic during row reduction. I ended up dropping this approach in favor of the current simulation rebuilding to avoid this: b9f03ca

There are two row reductions that currently happen:

  1. We need to solve for the differentials so we can apply an integration method (to get algebraic equations).
  2. We need to make the non-linear part of the system of (algebraic) equations small, for performance. This is really important, Newton's method on >50 variables is unfeasibly slow, Newton's method on 5-10 variables is fine.

(Background: http://dsharlet.com/2014/03/28/how-livespice-works-numerically-solving-circuit-equations/)

I think (1) is strictly necessary. However, I think (2) is (partially) not necessary, and even a bad idea sometimes. I think maybe we only need to partially row reduce to get the number of variables down to the number of non-linear equations. Currently, the whole system is row reduced such that after solving the non-linear system, there is only back substitution to do. But this might not be necessary, solving a linear system dynamically isn't bad (and could benefit a lot from SIMD), and might even have other benefits (we could choose pivots, which isn't possible symbolically).

The problem here is I'm not actually sure how to do this. It's not possible to simply row reduce anywhere we'd like, we need a pivot in every column we want to zero out, and you can't get that pivot by zeroing out every column before that.

We could just take the linear variables as the values from the previous timestep, solve the non-linear equations, and then get the new linear equations to solve. That would basically be a block-wise (with 2 blocks) Gauss-Seidel type thing. It seems a lot of hobbyist circuit simulators do something like this (usually without realizing it). I've generally tried to avoid Gauss-Seidel as it just seems sketchy, and should require more iterations (linear vs. quadratic convergence). But maybe this doesn't really matter in practice.

I think "real" large SPICE simulators use Gauss-Seidel a lot more, which would make sense (large sparse systems are going to be bad with Newton's method). But they're also a lot slower...

It's also possible that (1) by itself is enough to make dynamic controls infeasible.

@dsharlet
Copy link
Owner Author

dsharlet commented Nov 4, 2020

I just switched from the trapezoid method to BDF2 for numerical integration, which I just realized might help quite a bit with this. It cuts down a lot on the number of places the various functions that come out of analysis need to be evaluated, which is the main problem with dynamic parameters. Only reducing it by a factor of (strictly less than) 2 isn't going to solve it by itself, but maybe it will help...

@Federerer
Copy link
Collaborator

I've made some progress in this matter 😃
You can check a demo here
I've basically used your approach from b9f03ca which turned out to be very slow, as expected, but it was working fine for very small circuits, except that something was wrong (simulation was sometimes working, but potentiometers were amplifying signal 😕). It turned out to be a bug in Factor function in ComputerAlgebra\Extensions\Factor.cs which is sometimes randomly removing variables. Anyway, I commented out factorization and it's working 😄 And it's working faster than before - I suspect because the generated expressions are very repetitive and our current caching compiler handles it quite well. I'll test that with my decompilation trick later to confirm. There are some things that could be improved, as currently I'm updating the variables once per process function call, but you can extract all sub-expressions that depends only on those variables and calculate them only when parameter changes.

@Federerer
Copy link
Collaborator

Just pushed my POC here.

@dsharlet
Copy link
Owner Author

Wow, the demo results are incredible!! Very nice work.

The circuit you are simulating is even a reasonably complex circuit! I'm honestly shocked at how well this is working.

It's totally reasonable to remove Factor, aside from the bug, it makes sense that the compiler can do CSE better without it.

Have you found any circuits that perform worse with this? Is the solve time reasonable? But, really, since this change means that adjusting pots doesn't require solving the circuit again, even if it took multiple seconds, a long solve time would be OK!

This is a really great result, awesome work. I feel silly for not figuring this out when I was working on it :) Also, nice work on reviving the old branch, I remember attempting to revive this approach and getting bogged down on something. I think we should do a bit more testing and consider merging your POC, even if simulation performance regresses in some cases. This is a really great improvement, I really miss this approach where pots could be moved during simulation without reinitializing the simulation.

I see some seemingly spurious changes in your POC, e.g. swapping resistances and solving order for potentiometers - was this change necessary to get this working?

@dsharlet
Copy link
Owner Author

dsharlet commented May 10, 2022

@mikeoliphant we had looked at this a while back. @Federerer has made some real progress on getting this working here :)

@mikeoliphant
Copy link
Collaborator

Having to re-solve on parameter changes is a major limitation, so this would clearly be a big win.

If I recall correctly, I added some code in to delay parameter changing in the VST to keep it from updating the simulation too often. That could probably be removed if this fix is made to allow immediate continuous control.

@mikeoliphant
Copy link
Collaborator

This would also make it more practical to consider exposing parameters from the VST to the DAW (they currently are not) to allow for automation. The issue still remains, though, that the parameters change when you load a new circuit - which creates ugly problems with the DAW integration.

dsharlet added a commit that referenced this issue Dec 3, 2024
…mall change means that we don't need to solve for differentials before discretization, which may enable dynamic parameters to work! Partially addressing #94
@dsharlet
Copy link
Owner Author

dsharlet commented Dec 3, 2024

I have huge progress to report on this issue!!

It turns out that the row reduction (1) I described in the original report is actually not necessary, with a nearly trivial change that we even already had in the code, just commented out! 5398a63

I need to recognize Andy Simper of Cytomic (https://cytomic.com/, @andy-cytomic) for his contributions here. He reached out to discuss circuit simulation recently, and he pointed out that he doesn't do row reduction here. It turns out that the totally standard nodal analysis of capacitors avoids this, by inserting a new variable into the system to represent the current. LiveSPICE supports this, we just weren't doing it. The code to do it was even there in the code, just commented out! I commented it because it made things just slightly slower.

However, it's obviously worth eating that performance hit if it means we can support dynamic parameters properly.

I've done some testing with this branch (https://github.com/dsharlet/LiveSPICE/compare/dynamic-parameters), here is where things stand now:

  • Some real, major circuits can run successfully with dynamic parameters (e.g. Bassman preamp + tone stack).
  • Simulation solve time takes ~10x longer, but, I don't think this matters nearly as much any more, because changing a potentiometer does not require re-solving the circuit with this change.
  • Simulation performance drops from ~3x real time to ~2x real time for this example.
  • At least a few circuits that use to simulate successfully now do not. I suspect this is uncovering computer algebra bugs. If that's not the case, that means there are numerical stability issues that need to be worked out.

Next steps here:

  • I need to debug simulation stability issues for the circuits that have them.
  • I need to work on performance. I suspect much of the performance hits (both solve and sim speed) can be reduced. I already noticed one simple missed algebraic simplification in the output at one point.
  • The VST plugin needs updating for the new simulation interface. This won't be hard, just haven't gotten around to it yet.
  • ... ship it?

@dsharlet
Copy link
Owner Author

dsharlet commented Dec 3, 2024

To explain a bit further why that one-line uncommenting matters:

Currently, we get differential equations that look like this:

  D[VC1[t], t]*-1E-06 + i_1[t]==0
  D[VC1[t], t]*1E-06 - _v3[t]*0.0001 + _v4[t]*0.0001==0
  D[VCc[t], t]*5.1E-11 - _v18[t]*1.9607843137254903E-05 - _v19[t]*0.0002127659574468085 + _v5[t]*0.00023237380058406342 - iD1[t] + iD2[t]==0
  D[VCc[t], t]*-5.1E-11 - _v18[t]*2E-05 + _v6[t]*0.00102 - _v8[t]*0.001 + iD1[t] - iD2[t] + iX1[t]==0
  D[VCz[t], t]*4.7E-08 + _v19[t]*0.0002127659574468085 - _v5[t]*0.0002127659574468085==0
  D[VC4[t], t]*2.2E-07 - _v14[t]*0.0001 - _v21[t]*0.00010010010010010012 - _v6[t]*0.001 + _v8[t]*0.0012001001001001003==0
  D[VC6[t], t]*-1E-06 + _v12[t]*0.001 - _v20[t]*0.001 + iX4[t]==0
  D[VC5[t], t]*-2.2E-07 - _v20[t]*0.09999999999999991 + _v21[t]*0.10010010010010002 - _v8[t]*0.00010010010010010012==0
  D[VC5[t], t]*2.2E-07 - GND[t]*0.004545454545454545 + _v10[t]*0.004545454545454545==0
  D[VC6[t], t]*1E-06 - _v16[t]*2E-05 + _v17[t]*2E-05==0

This is not solvable, it has more equations than unknowns (the differentials). Row reduction fixes this.

However, row reduction has a horrible side effect of blowing up the expressions for the rest of the nodes in the circuit. When those nodes are mostly constants, it's a minor problem. However, if those expressions have symbols/variables like those from potentiometers, this is terrible. The expressions get so huge that the row reduction basically takes forever, I don't think I ever even bothered to let it finish on even modest circuits because it was impractically slow.

Now, if you apply 5398a63, the differential equations look like this:

  D[VC1[t], t]*-1E-06 + iC1[t]==0
  D[VC6[t], t]*-1E-06 + iC6[t]==0
  D[VC4[t], t]*-2.2E-07 + iC4[t]==0
  D[VC5[t], t]*-2.2E-07 + iC5[t]==0
  D[VCz[t], t]*-4.7E-08 + iCz[t]==0
  D[VCc[t], t]*-5.1E-11 + iCc[t]==0

No row reduction needed! In fact, I haven't even removed the row reduction step from the code, it's just automatically nearly a no-op now. We still need it basically just to sort the differentials to the top-left of the matrix, but that could easily be done more directly if needed.

Now we can let the expressions with dynamic parameter symbols/variables just fall through to the non-linear solver, which can handle them fine. It does slow down the simulation (linear variables are much faster to solve than non-linear during simulation), but I think it's worth the tradeoff, especially because I suspect more variables than necessary end up in the non-linear solver now.

@dsharlet
Copy link
Owner Author

dsharlet commented Dec 3, 2024

Another update: almost all of the numerical issues were just silly bugs about potentiometers getting too close to extreme values. Some code cleanups fix those.

The only remaining stability issue is one circuit that takes ages to solve (at least 10x longer than any other circuit), and when it finally does solve and run, it diverges.

Some circuits are actually faster on this branch, like the Phase 90, which now easily runs in real time. This used to be one of the toughest circuits!

@mikeoliphant
Copy link
Collaborator

Very cool!

@Federerer
Copy link
Collaborator

Nice, I have to check it on my branch ;)

@dsharlet
Copy link
Owner Author

dsharlet commented Dec 3, 2024

Here is the branch I'm working on: https://github.com/dsharlet/LiveSPICE/compare/dynamic-parameters

I think it's almost ready to merge. It has just one circuit in all of the tests that doesn't work, and I just want to try strengthening the computer algebra to simplify some of the new patterns resulting from this change, to hopefully recover some performance loss. But even the performance loss as it exists now is OK I think. I'm basically just hoping that cleaning up the math will somehow magically fix the broken circuit, or at least make it easier to debug.

@mikeoliphant
Copy link
Collaborator

Once you merge, I can update the VST to skip the re-solve on parameter change.

@dsharlet dsharlet linked a pull request Dec 5, 2024 that will close this issue
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 a pull request may close this issue.

3 participants