Closed samreid closed 2 years ago
I found that increasing ?minDT
from the default value of ?minDT=1E-3
to ?minDT=1E-2
corrected this problem. So it does seem like another occurrence of poor matrix conditioning in an LC circuit due to low dt values. However, we tried to turn the minDT
as low as we could in order to get accurate physics. We recently turned up minDT
because when it was too low we suffered from performance problems. I'm at a good point to check in with @arouinfar and @ariel-phet about what's possible.
@arouinfar and I discussed this today, we were able to reproduce the problem and see it disappear when dt was increased to 1E-2. However, changing the dt to 1E-2 causes some values to go past our previously determined error threshold of 0.05 in the unit tests. We discussed combining adjacent series inductors or capacitors into a single composite for purposes of the circuit solution--we were hopeful this may work because in the circuit above, we did not see a problem when we removed one inductor from the loop. However, this is a complex strategy that could take a week or so and would introduce questions like how to backpropagate solutions from the composite model back to individual circuit elements.
We also would like to see how the unit tests are failing when dt goes to 1E-2. If it is failing from a trailing or leading temporal problem, maybe it will be ok, since the user cannot start a clock in CCK within a fine resolution anyways. So I'll test that next and @arouinfar and I will discuss again shortly.
According to http://dev.hypertriton.com/edacious/trunk/doc/lec.pdf we can follow the Norton model for the inductor companion model to have all of the dt in the numerator. But we will have to add current sources. It's a current source in parallel with a resistor, with:
Req = dt/2/L
Ieq = i0 + Req * v0
This seems like the right thing to investigate next, since we would like to be able to turn down the dt without creating a poorly conditioned problem.
I implemented the norton inductor trapezoidal rule. Not sure about all the signs, but unit tests are failing.
To test:
I investigated all combinations of signs in this patch, none gave expected behavior (even ignoring unit tests signs). Next we should go back to the standard MNA and check all the signs in case that has an impact here.
@arouinfar and @ariel-phet and I worked on this today. We tried out the norton equivalent trapezoidal inductor and could not get it to work. We tried all combinations of signs and saw 3 behaviors:
We also saw that we need to seed the inductors with the DC operating values.
We also tried the forward euler, but could not look up the current through the resistor. We tried backward euler and had the same behavior as trapezoidal.
We discussed testing some of the buggy circumstances in the spice simulation. We tried running an RL circuit, but got the error Error: no data saved for Transient analysis; analysis not run
. We also discussed how we might wire up the spice simulator into the sim. This is discussed more in https://github.com/phetsims/circuit-construction-kit-common/issues/228
Page 25 of https://docs.lib.purdue.edu/cgi/viewcontent.cgi?article=1301&context=ecetr also has a treatment of inductor companion models. That page shows that for forward euler, there is no parallel resistor. I tried this:
And it is giving pretty good results in the unit test.
Here is the full patch with the forward euler norton inductor:
The values are coming out like this:
Expected Current | Actual Current |
---|---|
-0.07675913755469299 | -0.07852840330803694 |
-0.14173434471310536 | -0.1447233863638541 |
-0.1967346701436833 | -0.20052199676691962 |
-0.243291440483704 | -0.24755705560646554 |
-0.28270089574646085 | -0.2872049383056726 |
-0.31606027941427883 | -0.32062585115905434 |
-0.3442983880427012 | -0.3487977821654892 |
-0.36820143094213664 | -0.37211561741076454 |
-0.3884349199257851 | -0.39183746412946363 |
-0.40556219858121906 | -0.4088251266087866 |
-0.42006012696015305 | -0.42288577139755573 |
-0.43233235838169365 | -0.4347780366260082 |
-0.44272057800365616 | -0.44483632160429243 |
-0.45151401606779745 | -0.45334345584330443 |
-0.4589575006880506 | -0.460538651954452 |
-0.46525827438859924 | -0.46662423208324116 |
-0.47059176417878507 | -0.4717713170176646 |
-0.475106465816068 | -0.4761246379437748 |
-0.4789280782453618 | -0.4798160412860006 |
-0.4821630033263738 | -0.4829367115602657 |
-0.48490130828884076 | -0.48557489060975845 |
-0.4872192333967463 | -0.48780517708205207 |
-0.48918131464025344 | -0.4896906358227885 |
-0.4908421805556329 | -0.4912845811330362 |
-0.49224807320049535 | -0.49263208431471084 |
-0.4934381356315295 | -0.49377124813228396 |
-0.49444550173087887 | -0.4947394317149829 |
-0.4952982187242524 | -0.4955571229567258 |
-0.49602002807564677 | -0.496247714058219 |
-0.49663102650045726 | -0.4968309611875923 |
-0.4971482255009963 | -0.49732354965736497 |
I also tried putting an exponential damping term on the inductor voltage, and that seemed to correct some bad behavior:
Index: js/model/analysis/LTACircuit.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/model/analysis/LTACircuit.ts b/js/model/analysis/LTACircuit.ts
--- a/js/model/analysis/LTACircuit.ts (revision 0564d2ce40170627e3585b0438c57fe6664bf38f)
+++ b/js/model/analysis/LTACircuit.ts (date 1637120966663)
@@ -133,7 +133,7 @@
const newNode2 = 'syntheticNode' + ( syntheticNodeIndex++ );
const companionResistance = 2 * ltaInductor.inductance / dt;
- const companionVoltage = ltaInductor.voltage + -companionResistance * ltaInductor.current;
+ const companionVoltage = ( ltaInductor.voltage + -companionResistance * ltaInductor.current ) * 0.99;
const battery = new MNABattery( ltaInductor.node0, newNode, companionVoltage );
const resistor = new MNAResistor( newNode, newNode2, companionResistance );
It fixes the bad oscillation in this case:
Unit tests are all still within tolerance of 2% after this damping. 0.999 damping wasn't enough. 0.99 was enough. I didn't search between those values.
After the commit, the entire forward euler norton inductor is this (does not include the 1E-4 series resistance):
this.ltaInductors.forEach( ltaInductor => {
const companionCurrent = -dt * ltaInductor.voltage / ltaInductor.inductance + ltaInductor.current; // signs[ 0 ] * ltaInductor.current + signs[ 1 ] * companionResistance * ltaInductor.voltage;
companionCurrents.push( new MNACurrent( ltaInductor.node0, ltaInductor.node1, companionCurrent ) );
ltaInductor.inductorVoltageNode1 = ltaInductor.node1;
currentCompanions.push( {
element: ltaInductor,
getValueForSolution: ( solution: MNASolution ) => companionCurrent
} );
} );
Today, we observed that putting dt low enough, say to 1E-5, would correct the problem in the case described in https://github.com/phetsims/circuit-construction-kit-common/issues/780#issuecomment-971149638. We did not get the damping working.
However, we aren't sure how to pick a threshold for various architectures and lower inductance values. We observed that lower inductance values fail earlier. We also saw that inductors in Java go from 10 H to 100 H. In HTML5, we have been targeting between 0.1 H and 10 H. The question is how we can get resonance if we have the higher inductance values.
The resonant frequency is sqrt(1/LC)
and will need to match the driving frequency.
We want to hit all our learning goals about resonance, and weren’t sure what was possible. You can still use the same query parameters for changing the inductor min/max/default/step. And we added a decimal place (in master) to the ac frequency control for investigation.
At today's design meeting, we discussed that the range in java was 10H to 100H.
@ariel-phet: If you want to hit resonance at 10H, you need to reduce the capacitance accordingly. The capacitance ranges were already finely tuned. @kathy-phet: We need at least 1 way to hit resonance, but do not need to cover a large state space with resonances. Let's calculate where the resonance is. @ariel-phet: Can you please add capacitance range query parameters like we have for inductors? @samreid: Sure. @ariel-phet: There is a sweet spot for resonant frequency, we need to target that region. @arouinfar: Changing the capacitance will change the rate at which the plates saturate. @kathy-phet: I just want to understand if there are too many constraints to hit all of these different spaces. @ariel-phet: We really need to be able to hit resonance. @arouinfar: Maybe we will leave the default for capacitance as we have it now, but add a lower range for the resonance learning goals. @samreid: That sounds good to me. @ariel-phet: What about a screen that is all about resonance? Discussion about the plate distance. @ariel-phet so the proposal is to let the capacitance go to a lower range. Let's investigate that. @samreid: I'll add the query parameters to adjust those values. @kathy-phet : If L=5, we need C=0.005 to get f=1. Currently the min capacitance is 0.05. So it's just a factor of 10 less. @samreid: We will have to make sure the low capacitance doesn't give integration errors. @kathy-phet: Also, if resonance is the only feature we cannot solve soon (like within a week), then we may need to move forward without it, since so many other things are already working much better than before. @ariel-phet will also check Java to see what resonance was possible.
Next step is for @samreid to add capacitance range query parameters.
I added the query parameters, they will be ready for testing in the next dev version:
capacitanceMin: {
defaultValue: 0.05
},
capacitanceMax: {
defaultValue: 0.2
},
capacitanceStep: {
defaultValue: 0.001
},
capacitanceDefault: {
defaultValue: 0.1
},
capacitorNumberDecimalPlaces: {
defaultValue: 2
},
UPDATE: Please test in https://phet-dev.colorado.edu/html/circuit-construction-kit-ac/1.0.0-dev.51/phet/circuit-construction-kit-ac_en_phet.html
I thought after the last group meeting I attended to discuss this, we decided to stick with the original values @ariel-phet had chosen and then restrict to one inductor. So you can hit resonance with the L values Ariel chose a while back.
I thought any revisiting of values was going to happen - as needed - after publishing 1.0, when we continue to diagnose and figure out if we can support a version with multiple inductors allowed.
So, bottom line, my recommendation is to 1) Keep these query parameters in so that we can investigate 2) But not to make this a blocking issue. Use the values that Ariel had found for L that allow access to resonance for now, and the values of R and C that we have had for a long time now.
@arouinfar - Do you agree with this recollection (see ^^)
@kathy-phet that was my understanding, that since we were going to limit to 1 inductor for the moment, no need to mess around with the current default values of L and C if we can avoid it.
@arouinfar and I cleared the milestone since it seems we don't need to change the capacitor ranges or anything else for RC.1. @arouinfar recommended closing this issue.
In https://github.com/phetsims/circuit-construction-kit-common/issues/772#issuecomment-965690073 @stemilymill said:
For https://github.com/phetsims/qa/issues/736
I'm not sure if this is helpful but it seems related:
Several times I was able to freeze the sim in the manner shown in this video. You can still see the current still flowing here but the sim is not responsive to user input and eventually completely crashes. This was taken with a very low frame rate, and I sped up the beginning (building the circuit) so the choppiness is not actual performance. I can work on getting a higher framerate recording.
https://user-images.githubusercontent.com/85511101/141181334-c6f1c341-487e-4651-b6fb-3dc4f92b4bae.mp4