Bad voltage jitter in driven resonant circuit

[ariel-phet]

Seen as part of testing in https://github.com/phetsims/circuit-construction-kit-common/issues/748

In the driven resonant circuit seen below, I was getting some nasty jitter in the voltage readout, but the current readout is clean.

Capacitance 0.10 F Inductance 0.253 H Resistance 40 Ohm AC source 100 V and 1 Hz

These values of capacitance and inductance were initially chosen to set up a natural frequency of 1 Hz. Adjusting the voltage does not improve the behavior much, nor does adjusting the driving frequency. Lowering the resistance does eventually clean up the voltage readout, but jitter can easily be seen as low as 10 Ohms in this circuit. Increasing the inductance to about 1.5 H also cleans up the behavior. But clearly there is currently a widely accessible range of values that produces poor behavior.

[arouinfar]

Wow, that looks awful and is easily reproducible in macOS 10.15.7/Chrome.

It reminds me a bit of the jagged graphs we saw in https://github.com/phetsims/circuit-construction-kit-common/issues/720

[samreid]

I experimented with reducing the MIN_DT and ERROR_THRESHOLD in TimestepSubdivisions.ts. However, reducing it lower and lower wrecked the performance before the curve smoothed out appreciably. This is an emergent behavior from our solver and I'm not sure how to investigate it next. @arouinfar or @ariel-phet can you brainstorm ideas where to look? Or workarounds we could explore? A smoothing step in the graph? Limiting the sliders to keep us out of troublesome areas?

I could also print out the equations and condition matrix at those trouble areas to help us try to understand what is wrong.

[samreid]

This case is not a problem for Java. I edited the Java to take the inductor below its default of 10, set up the same characteristics and it did OK. But it did print out "Time step too small" over and over:

[samreid]

This patch is smoother and seems to have qualitatively correct behavior for the battery-resistor-inductor circuit:

```diff Index: js/model/mna/InductorAdapter.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/model/mna/InductorAdapter.ts b/js/model/mna/InductorAdapter.ts --- a/js/model/mna/InductorAdapter.ts (revision 34562797df6b4f3d86b184ef75202eeca8089f20) +++ b/js/model/mna/InductorAdapter.ts (date 1634711559969) @@ -32,7 +32,7 @@ * @public */ applySolution( circuitResult: CircuitResult ) { - this.inductor.currentProperty.value = -circuitResult.getTimeAverageCurrent( this.dynamicCircuitInductor ); + this.inductor.currentProperty.value = circuitResult.getTimeAverageCurrent( this.dynamicCircuitInductor ); this.inductor.mnaCurrent = CCKCUtils.clampMagnitude( circuitResult.getInstantaneousCurrent( this.dynamicCircuitInductor ) ); this.inductor.mnaVoltageDrop = CCKCUtils.clampMagnitude( circuitResult.getInstantaneousVoltage( this.dynamicCircuitInductor ) ); assert && assert( Math.abs( this.inductor.mnaCurrent ) < 1E100, 'mnaCurrent out of range' ); Index: js/model/mna/CapacitorAdapter.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/model/mna/CapacitorAdapter.ts b/js/model/mna/CapacitorAdapter.ts --- a/js/model/mna/CapacitorAdapter.ts (revision 34562797df6b4f3d86b184ef75202eeca8089f20) +++ b/js/model/mna/CapacitorAdapter.ts (date 1634744797931) @@ -42,8 +42,7 @@ ( typeof this.capacitorVoltageNode0 === 'number' || typeof this.capacitorVoltageNode0 === 'string' ) && ( typeof this.capacitorVoltageNode1 === 'number' || typeof this.capacitorVoltageNode1 === 'string' ) ) { - this.capacitor.mnaVoltageDrop = CCKCUtils.clampMagnitude( circuitResult.getFinalState().dynamicCircuitSolution!.getNodeVoltage( this.capacitorVoltageNode1 ) - - circuitResult.getFinalState().dynamicCircuitSolution!.getNodeVoltage( this.capacitorVoltageNode0 ) ); + this.capacitor.mnaVoltageDrop = CCKCUtils.clampMagnitude( circuitResult.getInstantaneousVoltage( this.dynamicCircuitCapacitor ) ); } assert && assert( Math.abs( this.capacitor.mnaCurrent ) < 1E100, 'mnaCurrent out of range' ); Index: js/model/mna/DynamicCircuit.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/model/mna/DynamicCircuit.ts b/js/model/mna/DynamicCircuit.ts --- a/js/model/mna/DynamicCircuit.ts (revision 34562797df6b4f3d86b184ef75202eeca8089f20) +++ b/js/model/mna/DynamicCircuit.ts (date 1634745340084) @@ -137,7 +137,7 @@ syntheticNodeIndex++; const companionResistance = 2 * inductor.inductance / dt; - const companionVoltage = -inductorAdapter.state.voltage - companionResistance * inductorAdapter.state.current; + const companionVoltage = inductorAdapter.state.voltage + companionResistance * inductorAdapter.state.current; const battery = new ModifiedNodalAnalysisCircuitElement( inductor.nodeId0, newNode, null, companionVoltage ); const resistor = new ModifiedNodalAnalysisCircuitElement( newNode, inductor.nodeId1, null, companionResistance ); @@ -148,7 +148,7 @@ // in series, so current is same through both companion components currentCompanions.push( { element: inductorAdapter, - getValueForSolution: ( solution: ModifiedNodalAnalysisSolution ) => -solution.getCurrentForResistor( resistor ) + getValueForSolution: ( solution: ModifiedNodalAnalysisSolution ) => solution.getCurrentForResistor( resistor ) } ); } ); @@ -235,7 +235,7 @@ } ); const updatedInductors = this.inductorAdapters.map( inductorAdapter => { const newState = new DynamicElementState( - solution.getNodeVoltage( inductorAdapter.dynamicCircuitInductor.nodeId1 ) - solution.getNodeVoltage( inductorAdapter.dynamicCircuitInductor.nodeId0 ), + -solution.getNodeVoltage( inductorAdapter.dynamicCircuitInductor.nodeId1 ) - solution.getNodeVoltage( inductorAdapter.dynamicCircuitInductor.nodeId0 ), solution.getCurrent( inductorAdapter ) ); return new DynamicInductor( inductorAdapter.dynamicCircuitInductor, newState ); ```

Still has a hitch every now and then but overall is smoother. Summary of the patch: sign changes and different way of computing voltage:

[samreid]

At today's design meeting, we saw that the jitter was only in the voltage across the inductor and not across any of the other components, or in any of the currents. @ariel-phet and I scheduled a meeting to take a closer look.

[samreid]

@ariel-phet and I identified this suspicious part:

   const updatedCapacitors = this.dynamicCapacitors.map( capacitorAdapter => {
      const newState = new DynamicElementState(
        // TODO: This may have something to do with it?  https://github.com/phetsims/circuit-construction-kit-common/issues/758
        solution.getNodeVoltage( capacitorAdapter.capacitorVoltageNode1! ) - solution.getNodeVoltage( capacitorAdapter.capacitorVoltageNode0! ),
        solution.getCurrent( capacitorAdapter )
      );
      return new DynamicCapacitor( capacitorAdapter.dynamicCircuitCapacitor, newState );
    } );
    const updatedInductors = this.dynamicInductors.map( inductorAdapter => {
      const newState = new DynamicElementState(
        // TODO: This may have something to do with it? https://github.com/phetsims/circuit-construction-kit-common/issues/758
        solution.getNodeVoltage( inductorAdapter.dynamicCircuitInductor.nodeId1 ) - solution.getNodeVoltage( inductorAdapter.dynamicCircuitInductor.nodeId0 ),
        solution.getCurrent( inductorAdapter )
      );
      return new DynamicInductor( inductorAdapter.dynamicCircuitInductor, newState );
    } );

Note that the top part uses capacitorAdapter.capacitorVoltageNode1 but the bottom part uses inductorAdapter.dynamicCircuitInductor.nodeId1. This is asymmetrical and may indicate a problem.

[samreid]

@ariel-phet and I noticed this other question, why is the sign negative at node 1 and positive at node 0?

          // Each resistor with nonzero resistance has unknown voltages
          nodeTerms.push( new Term( -sign / resistor.resistanceValue, new UnknownVoltage( resistor.nodeId1 ) ) );
          nodeTerms.push( new Term( sign / resistor.resistanceValue, new UnknownVoltage( resistor.nodeId0 ) ) );

[samreid]

@ariel-phet and I noticed this asymmetrical code that ResistiveBatteryadapter is not put into the main list. Here is after our change:

DynamicCircuit line 161 or so.

    const newBatteryList = [ ...this.resistiveBatteryAdapters, ...companionBatteries ];
    const newResistorList = [ ...this.resistorAdapters, ...companionResistors ];
    // const newCurrentList: MNACurrentSource[] = []; // Placeholder for if we add other circuit elements in the future

    const mnaCircuit = new ModifiedNodalAnalysisCircuit( newBatteryList, newResistorList );

[samreid]

@ariel-phet recommends getting rid of current sources if they are currently unused. But maybe we should keep notes on how it is done, in case we need that for companion models for the future. @ariel-phet thinks it is unlikely we would add current sources to the toolbox.

There is something odd about the inductor model. We tried simulating it as a 10V battery, but it kept coming out as a 5V battery (but only for the first inductor). We also need to understand why https://github.com/phetsims/circuit-construction-kit-common/issues/762 is causing different physics. Then the problems seem isolated to the inductor, so we can take a closer look into it.

[samreid]

Patch of all working copy changes from collaboration with @ariel-phet:

```diff Index: js/model/mna/ModifiedNodalAnalysisCircuit.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/model/mna/ModifiedNodalAnalysisCircuit.ts b/js/model/mna/ModifiedNodalAnalysisCircuit.ts --- a/js/model/mna/ModifiedNodalAnalysisCircuit.ts (revision d46d5cc8c04a337fbcf9818df55ccf277a0dfd41) +++ b/js/model/mna/ModifiedNodalAnalysisCircuit.ts (date 1634928284346) @@ -19,11 +19,14 @@ import circuitConstructionKitCommon from '../../circuitConstructionKitCommon.js'; import ModifiedNodalAnalysisSolution from './ModifiedNodalAnalysisSolution.js'; import ModifiedNodalAnalysisCircuitElement from './ModifiedNodalAnalysisCircuitElement.js'; +import MNABattery from './MNABattery.js'; +import MNAResistor from './MNAResistor.js'; +// import MNACurrentSource from './MNACurrentSource.js'; class ModifiedNodalAnalysisCircuit { - private readonly batteries: ModifiedNodalAnalysisCircuitElement[]; - private readonly resistors: ModifiedNodalAnalysisCircuitElement[]; - private readonly currentSources: ModifiedNodalAnalysisCircuitElement[]; + private readonly batteries: MNABattery[]; + private readonly resistors: MNAResistor[]; + // private readonly currentSources: MNACurrentSource[]; private readonly elements: ModifiedNodalAnalysisCircuitElement[]; private readonly nodeSet: { [ key: string ]: string }; private readonly nodeCount: number; @@ -32,12 +35,11 @@ /** * @param {ModifiedNodalAnalysisCircuitElement[]} batteries * @param {ModifiedNodalAnalysisCircuitElement[]} resistors - * @param {ModifiedNodalAnalysisCircuitElement[]} currentSources */ - constructor( batteries: ModifiedNodalAnalysisCircuitElement[], resistors: ModifiedNodalAnalysisCircuitElement[], currentSources: ModifiedNodalAnalysisCircuitElement[] ) { + constructor( batteries: MNABattery[], resistors: MNAResistor[] ) { assert && assert( batteries, 'batteries should be defined' ); assert && assert( resistors, 'resistors should be defined' ); - assert && assert( currentSources, 'currentSources should be defined' ); + // assert && assert( currentSources, 'currentSources should be defined' ); // @public (read-only) {ModifiedNodalAnalysisCircuitElement[]} this.batteries = batteries; @@ -46,10 +48,11 @@ this.resistors = resistors; // @public (read-only) {ModifiedNodalAnalysisCircuitElement[]} - this.currentSources = currentSources; + // this.currentSources = currentSources; // @public (read-only) {ModifiedNodalAnalysisCircuitElement[]} - the list of all the elements for ease of access - this.elements = this.batteries.concat( this.resistors ).concat( this.currentSources ); + // @ts-ignore + this.elements = this.batteries.concat( this.resistors ); // @public (read-only) {Object} - an object with index for all keys that have a node in the circuit, such as: // {0:0, 1:1, 2:2, 7:7} @@ -76,9 +79,7 @@ */ toString() { if ( assert ) { // stripped out for builds - return `resistors:\n${this.resistors.map( resistorToString ).join( '\n' )}\n` + - `batteries:\n${this.batteries.map( batteryToString ).join( '\n' )}\n` + - `currentSources:\n${this.currentSources.map( c => c.toString() ).join( '\n' )}`; + return 'test'; } else { return 'toString() only defined when assertions are enabled'; @@ -94,7 +95,7 @@ getCurrentCount() { let numberOfResistanceFreeResistors = 0; for ( let i = 0; i < this.resistors.length; i++ ) { - if ( this.resistors[ i ].mnaValue === 0 ) { + if ( this.resistors[ i ].resistanceValue === 0 ) { numberOfResistanceFreeResistors++; } } @@ -119,19 +120,19 @@ */ getCurrentSourceTotal( nodeIndex: string ) { let currentSourceTotal = 0.0; - for ( let i = 0; i < this.currentSources.length; i++ ) { - const currentSource = this.currentSources[ i ]; - if ( currentSource.nodeId1 === nodeIndex ) { - - // positive current is entering the node, and the convention is for incoming current to be negative - currentSourceTotal = currentSourceTotal - currentSource.mnaValue; - } - if ( currentSource.nodeId0 === nodeIndex ) { - - // positive current is leaving the node, and the convention is for outgoing current to be positive - currentSourceTotal = currentSourceTotal + currentSource.mnaValue; - } - } + // for ( let i = 0; i < this.currentSources.length; i++ ) { + // const currentSource = this.currentSources[ i ]; + // if ( currentSource.nodeId1 === nodeIndex ) { + // + // // positive current is entering the node, and the convention is for incoming current to be negative + // currentSourceTotal = currentSourceTotal - currentSource.currentValue; + // } + // if ( currentSource.nodeId0 === nodeIndex ) { + // + // // positive current is leaving the node, and the convention is for outgoing current to be positive + // currentSourceTotal = currentSourceTotal + currentSource.currentValue; + // } + // } return currentSourceTotal; } @@ -158,7 +159,7 @@ const resistor = this.resistors[ i ]; if ( resistor[ side ] === node ) { - if ( resistor.mnaValue === 0 ) { + if ( resistor.resistanceValue === 0 ) { // Each resistor with 0 resistance introduces an unknown current, and v1=v2 nodeTerms.push( new Term( sign, new UnknownCurrent( resistor ) ) ); @@ -166,8 +167,8 @@ else { // Each resistor with nonzero resistance has unknown voltages - nodeTerms.push( new Term( -sign / resistor.mnaValue, new UnknownVoltage( resistor.nodeId1 ) ) ); - nodeTerms.push( new Term( sign / resistor.mnaValue, new UnknownVoltage( resistor.nodeId0 ) ) ); + nodeTerms.push( new Term( -sign / resistor.resistanceValue, new UnknownVoltage( resistor.nodeId1 ) ) ); + nodeTerms.push( new Term( sign / resistor.resistanceValue, new UnknownVoltage( resistor.nodeId0 ) ) ); } } } @@ -254,13 +255,13 @@ this.getCurrentTerms( node, 'nodeId1', -1, currentTerms ); this.getCurrentTerms( node, 'nodeId0', +1, currentTerms ); - equations.push( new Equation( this.getCurrentSourceTotal( node ), currentTerms ) ); + equations.push( new Equation( 0, currentTerms ) ); } // For each battery, voltage drop is given for ( let i = 0; i < this.batteries.length; i++ ) { const battery = this.batteries[ i ]; - equations.push( new Equation( battery.mnaValue, [ + equations.push( new Equation( battery.voltageValue, [ new Term( -1, new UnknownVoltage( battery.nodeId0 ) ), new Term( 1, new UnknownVoltage( battery.nodeId1 ) ) ] ) ); @@ -269,7 +270,7 @@ // If resistor has no resistance, nodeId0 and nodeId1 should have same voltage for ( let i = 0; i < this.resistors.length; i++ ) { const resistor = this.resistors[ i ]; - if ( resistor.mnaValue === 0 ) { + if ( resistor.resistanceValue === 0 ) { equations.push( new Equation( 0, [ new Term( 1, new UnknownVoltage( resistor.nodeId0 ) ), new Term( -1, new UnknownVoltage( resistor.nodeId1 ) ) @@ -293,9 +294,9 @@ unknownCurrents.push( new UnknownCurrent( this.batteries[ i ] ) ); } - // Treat resisters with R=0 as having unknown current and v1=v2 + // Treat resistors with R=0 as having unknown current and v1=v2 for ( let i = 0; i < this.resistors.length; i++ ) { - if ( this.resistors[ i ].mnaValue === 0 ) { + if ( this.resistors[ i ].resistanceValue === 0 ) { unknownCurrents.push( new UnknownCurrent( this.resistors[ i ] ) ); } } @@ -393,16 +394,16 @@ * @param {Resistor} resistor * @returns {string} */ -const resistorToString = ( resistor: ModifiedNodalAnalysisCircuitElement ) => - `node${resistor.nodeId0} -> node${resistor.nodeId1} @ ${resistor.mnaValue} Ohms`; +const resistorToString = ( resistor: MNAResistor ) => + `node${resistor.nodeId0} -> node${resistor.nodeId1} @ ${resistor.resistanceValue} Ohms`; /** * For debugging, display a Battery as a string * @param {Battery} battery * @returns {string} */ -const batteryToString = ( battery: ModifiedNodalAnalysisCircuitElement ) => - `node${battery.nodeId0} -> node${battery.nodeId1} @ ${battery.mnaValue} Volts`; +const batteryToString = ( battery: MNABattery ) => + `node${battery.nodeId0} -> node${battery.nodeId1} @ ${battery.voltageValue} Volts`; class Term { readonly coefficient: number; Index: js/model/mna/ResistorAdapter.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/model/mna/ResistorAdapter.ts b/js/model/mna/ResistorAdapter.ts --- a/js/model/mna/ResistorAdapter.ts (revision d46d5cc8c04a337fbcf9818df55ccf277a0dfd41) +++ b/js/model/mna/ResistorAdapter.ts (date 1634927359856) @@ -1,11 +1,11 @@ // Copyright 2021, University of Colorado Boulder -import ModifiedNodalAnalysisCircuitElement from './ModifiedNodalAnalysisCircuitElement.js'; import Circuit from '../Circuit.js'; import CircuitResult from './CircuitResult.js'; import CircuitElement from '../CircuitElement.js'; +import MNAResistor from './MNAResistor.js'; -class ResistorAdapter extends ModifiedNodalAnalysisCircuitElement { +class ResistorAdapter extends MNAResistor { /** * @param {Circuit} circuit Index: js/model/mna/DynamicCircuitSolution.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/model/mna/DynamicCircuitSolution.ts b/js/model/mna/DynamicCircuitSolution.ts --- a/js/model/mna/DynamicCircuitSolution.ts (revision d46d5cc8c04a337fbcf9818df55ccf277a0dfd41) +++ b/js/model/mna/DynamicCircuitSolution.ts (date 1634927359853) @@ -6,6 +6,7 @@ import DynamicInductorAdapter from './DynamicInductorAdapter.js'; import DynamicCapacitor from './DynamicCapacitor.js'; import DynamicInductor from './DynamicInductor.js'; +import MNAResistor from './MNAResistor.js'; class DynamicCircuitSolution { private readonly circuit: DynamicCircuit; @@ -65,8 +66,8 @@ } else { - assert && assert( element instanceof ModifiedNodalAnalysisCircuitElement ); - return this.mnaSolution.getCurrentForResistor( element as ModifiedNodalAnalysisCircuitElement ); + assert && assert( element instanceof MNAResistor ); + return this.mnaSolution.getCurrentForResistor( element as MNAResistor ); } } Index: js/model/mna/ModifiedNodalAnalysisSolution.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/model/mna/ModifiedNodalAnalysisSolution.ts b/js/model/mna/ModifiedNodalAnalysisSolution.ts --- a/js/model/mna/ModifiedNodalAnalysisSolution.ts (revision d46d5cc8c04a337fbcf9818df55ccf277a0dfd41) +++ b/js/model/mna/ModifiedNodalAnalysisSolution.ts (date 1634927359849) @@ -8,6 +8,7 @@ import circuitConstructionKitCommon from '../../circuitConstructionKitCommon.js'; import ModifiedNodalAnalysisCircuitElement from './ModifiedNodalAnalysisCircuitElement.js'; +import MNAResistor from './MNAResistor.js'; class ModifiedNodalAnalysisSolution { private readonly nodeVoltages: { [ key: string ]: number }; @@ -103,8 +104,8 @@ * @returns {number} * @public */ - getCurrentForResistor( resistor: ModifiedNodalAnalysisCircuitElement ) { - assert && assert( resistor.mnaValue > 0, 'resistor must have resistance to use Ohms Law' ); + getCurrentForResistor( resistor: MNAResistor ) { + assert && assert( resistor.resistanceValue > 0, 'resistor must have resistance to use Ohms Law' ); // To help understand the minus sign here: // Imagine a resistor that goes from node r0 to r1, with a conventional current flowing from r0 to r1. Then @@ -113,7 +114,7 @@ // Conversely, if v0>v1, then voltage is negative, so for the conventional current to flow to the right we must // multiply it by a negative. // Same sign as Java, see https://github.com/phetsims/circuit-construction-kit-common/issues/758 - return -this.getVoltage( resistor ) / resistor.mnaValue; + return -this.getVoltage( resistor ) / resistor.resistanceValue; } /** Index: js/model/mna/ModifiedNodalAnalysisAdapter.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/model/mna/ModifiedNodalAnalysisAdapter.ts b/js/model/mna/ModifiedNodalAnalysisAdapter.ts --- a/js/model/mna/ModifiedNodalAnalysisAdapter.ts (revision d46d5cc8c04a337fbcf9818df55ccf277a0dfd41) +++ b/js/model/mna/ModifiedNodalAnalysisAdapter.ts (date 1634927359852) @@ -141,8 +141,8 @@ const coefficient = 3; // shift by base so at V=0 the log is 1 - resistorAdapter.mnaValue = 10 + coefficient * V / logWithBase( V + base, base ); - resistorAdapter.circuitElement.resistanceProperty.value = resistorAdapter.mnaValue; + resistorAdapter.resistanceValue = 10 + coefficient * V / logWithBase( V + base, base ); + resistorAdapter.circuitElement.resistanceProperty.value = resistorAdapter.resistanceValue; } } ); Index: js/model/mna/DynamicCircuit.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/model/mna/DynamicCircuit.ts b/js/model/mna/DynamicCircuit.ts --- a/js/model/mna/DynamicCircuit.ts (revision d46d5cc8c04a337fbcf9818df55ccf277a0dfd41) +++ b/js/model/mna/DynamicCircuit.ts (date 1634929615463) @@ -12,7 +12,6 @@ import CircuitResult from './CircuitResult.js'; import circuitConstructionKitCommon from '../../circuitConstructionKitCommon.js'; import ModifiedNodalAnalysisCircuit from './ModifiedNodalAnalysisCircuit.js'; -import ModifiedNodalAnalysisCircuitElement from './ModifiedNodalAnalysisCircuitElement.js'; import TimestepSubdivisions from './TimestepSubdivisions.js'; import DynamicCircuitSolution from './DynamicCircuitSolution.js'; import DynamicState from './DynamicState.js'; @@ -21,9 +20,13 @@ import DynamicCapacitor from './DynamicCapacitor.js'; import ModifiedNodalAnalysisSolution from './ModifiedNodalAnalysisSolution.js'; import DynamicCircuitResistiveBattery from './DynamicCircuitResistiveBattery.js'; +import MNAResistor from './MNAResistor.js'; +import MNABattery from './MNABattery.js'; + +// import MNACurrentSource from './MNACurrentSource.js'; class DynamicCircuit { - private readonly resistorAdapters: ModifiedNodalAnalysisCircuitElement[]; + private readonly resistorAdapters: MNAResistor[]; private readonly resistiveBatteryAdapters: DynamicCircuitResistiveBattery[]; readonly dynamicCapacitors: DynamicCapacitor[]; readonly dynamicInductors: DynamicInductor[]; @@ -34,7 +37,7 @@ * @param {DynamicCapacitor[]} dynamicCapacitors * @param {DynamicInductor[]} dynamicInductors */ - constructor( resistorAdapters: ModifiedNodalAnalysisCircuitElement[], + constructor( resistorAdapters: MNAResistor[], resistiveBatteryAdapters: DynamicCircuitResistiveBattery[], dynamicCapacitors: DynamicCapacitor[], dynamicInductors: DynamicInductor[] ) { @@ -55,8 +58,8 @@ */ solvePropagate( dt: number ) { - const companionBatteries: ModifiedNodalAnalysisCircuitElement[] = []; - const companionResistors: ModifiedNodalAnalysisCircuitElement[] = []; + const companionBatteries: MNABattery[] = []; + const companionResistors: MNAResistor[] = []; const currentCompanions: { element: any, getValueForSolution: any }[] = []; // Node indices that have been used @@ -67,8 +70,8 @@ const newNode = 'syntheticNode' + syntheticNodeIndex; syntheticNodeIndex++; - const idealBattery = new ModifiedNodalAnalysisCircuitElement( resistiveBatteryAdapter.nodeId0 + '', newNode, null, resistiveBatteryAdapter.voltage ); // final LinearCircuitSolver.Battery - const idealResistor = new ModifiedNodalAnalysisCircuitElement( newNode, resistiveBatteryAdapter.nodeId1 + '', null, resistiveBatteryAdapter.resistance ); // LinearCircuitSolver.Resistor + const idealBattery = new MNABattery( resistiveBatteryAdapter.nodeId0 + '', newNode, null, resistiveBatteryAdapter.voltage ); // final LinearCircuitSolver.Battery + const idealResistor = new MNAResistor( newNode, resistiveBatteryAdapter.nodeId1 + '', null, resistiveBatteryAdapter.resistance ); // LinearCircuitSolver.Resistor companionBatteries.push( idealBattery ); companionResistors.push( idealResistor ); @@ -105,9 +108,9 @@ // |V|=|IReq| and sign is unchanged since the conventional current flows from high to low voltage. const companionVoltage = dynamicCapacitor.state.voltage - companionResistance * dynamicCapacitor.state.current; - const battery = new ModifiedNodalAnalysisCircuitElement( dynamicCapacitor.dynamicCircuitCapacitor.nodeId0 + '', newNode1, null, companionVoltage ); - const resistor = new ModifiedNodalAnalysisCircuitElement( newNode1, newNode2, null, companionResistance ); - const resistor2 = new ModifiedNodalAnalysisCircuitElement( newNode2, dynamicCapacitor.dynamicCircuitCapacitor.nodeId1 + '', null, resistanceTerm ); + const battery = new MNABattery( dynamicCapacitor.dynamicCircuitCapacitor.nodeId0 + '', newNode1, null, companionVoltage ); + const resistor = new MNAResistor( newNode1, newNode2, null, companionResistance ); + const resistor2 = new MNAResistor( newNode2, dynamicCapacitor.dynamicCircuitCapacitor.nodeId1 + '', null, resistanceTerm ); companionBatteries.push( battery ); companionResistors.push( resistor ); @@ -135,12 +138,14 @@ syntheticNodeIndex++; const companionResistance = 2 * inductor.inductance / dt; + const companionVoltage = -dynamicInductor.state.voltage - companionResistance * dynamicInductor.state.current; // TODO: this is how it appears in Java https://github.com/phetsims/circuit-construction-kit-common/issues/758 // const companionVoltage = dynamicInductor.state.voltage + companionResistance * dynamicInductor.state.current; + // debugger; - const battery = new ModifiedNodalAnalysisCircuitElement( inductor.nodeId0 + '', newNode, null, companionVoltage ); - const resistor = new ModifiedNodalAnalysisCircuitElement( newNode, inductor.nodeId1 + '', null, companionResistance ); + const battery = new MNABattery( inductor.nodeId0 + '', newNode, null, companionVoltage ); + const resistor = new MNAResistor( newNode, inductor.nodeId1 + '', null, companionResistance ); companionBatteries.push( battery ); companionResistors.push( resistor ); @@ -149,15 +154,15 @@ currentCompanions.push( { element: dynamicInductor, // TODO: This sign looks very wrong https://github.com/phetsims/circuit-construction-kit-common/issues/758 - getValueForSolution: ( solution: ModifiedNodalAnalysisSolution ) => -solution.getCurrentForResistor( resistor ) + getValueForSolution: ( solution: ModifiedNodalAnalysisSolution ) => solution.getCurrentForResistor( resistor ) } ); } ); const newBatteryList = companionBatteries; const newResistorList = [ ...this.resistorAdapters, ...companionResistors ]; - const newCurrentList: ModifiedNodalAnalysisCircuitElement[] = []; // Placeholder for if we add other circuit elements in the future + // const newCurrentList: MNACurrentSource[] = []; // Placeholder for if we add other circuit elements in the future - const mnaCircuit = new ModifiedNodalAnalysisCircuit( newBatteryList, newResistorList, newCurrentList ); + const mnaCircuit = new ModifiedNodalAnalysisCircuit( newBatteryList, newResistorList ); const mnaSolution = mnaCircuit.solve(); return new DynamicCircuitSolution( this, mnaSolution, currentCompanions ); Index: js/model/mna/ModifiedNodalAnalysisCircuitTests.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/model/mna/ModifiedNodalAnalysisCircuitTests.ts b/js/model/mna/ModifiedNodalAnalysisCircuitTests.ts --- a/js/model/mna/ModifiedNodalAnalysisCircuitTests.ts (revision d46d5cc8c04a337fbcf9818df55ccf277a0dfd41) +++ b/js/model/mna/ModifiedNodalAnalysisCircuitTests.ts (date 1634928284338) @@ -6,215 +6,12 @@ * @author Sam Reid (PhET Interactive Simulations) */ -import ModifiedNodalAnalysisCircuit from './ModifiedNodalAnalysisCircuit.js'; -import ModifiedNodalAnalysisCircuitElement from './ModifiedNodalAnalysisCircuitElement.js'; -import ModifiedNodalAnalysisSolution from './ModifiedNodalAnalysisSolution.js'; +// import MNABattery from './MNABattery.js'; +// import MNAResistor from './MNAResistor.js'; +// import ModifiedNodalAnalysisCircuit from './ModifiedNodalAnalysisCircuit.js'; +// import ModifiedNodalAnalysisSolution from './ModifiedNodalAnalysisSolution.js'; +// import MNACurrentSource from './MNACurrentSource.js'; QUnit.module( 'ModifiedNodalAnalysisCircuitTests' ); -const approxEquals = ( a: number, b: number ) => Math.abs( a - b ) < 1E-6; - -QUnit.test( 'test_battery_resistor_circuit_should_have_correct_voltages_and_currents_for_a_simple_circuit', - assert => { - const battery = new ModifiedNodalAnalysisCircuitElement( '0', '1', null, 4.0 ); - const resistor = new ModifiedNodalAnalysisCircuitElement( '1', '0', null, 4.0 ); - const circuit = new ModifiedNodalAnalysisCircuit( [ battery ], [ resistor ], [] ); - const voltageMap = { 0: 0.0, 1: 4.0 }; - - const desiredSolution = new ModifiedNodalAnalysisSolution( voltageMap, [ battery ] ); - const solution = circuit.solve(); - assert.equal( true, solution.approxEquals( desiredSolution, assert ), 'solutions instances should match' ); - - const currentThroughResistor = solution.getCurrentForResistor( resistor ); - - // should be flowing forward through resistor - assert.equal( approxEquals( currentThroughResistor, 1.0 ), true, 'current should be 1 amp through the resistor' ); - } ); - -QUnit.test( 'test_battery_resistor_circuit_should_have_correct_voltages_and_currents_for_a_simple_circuit_ii', - assert => { - const battery = new ModifiedNodalAnalysisCircuitElement( '0', '1', null, 4.0 ); - const resistor = new ModifiedNodalAnalysisCircuitElement( '1', '0', null, 2.0 ); - const circuit = new ModifiedNodalAnalysisCircuit( [ battery ], [ resistor ], [] ); - const desiredSolution = new ModifiedNodalAnalysisSolution( { - 0: 0, - 1: 4 - }, [ battery.withCurrentSolution( 2.0 ) ] ); - const solution = circuit.solve(); - assert.equal( solution.approxEquals( desiredSolution, assert ), true, 'solution should match' ); - } ); - - -QUnit.test( 'test_should_be_able_to_obtain_current_for_a_resistor', assert => { - const battery = new ModifiedNodalAnalysisCircuitElement( '0', '1', null, 4.0 ); - const resistor = new ModifiedNodalAnalysisCircuitElement( '1', '0', null, 2.0 ); - const solution = new ModifiedNodalAnalysisCircuit( [ battery ], [ resistor ], [] ).solve(); - const desiredSolution = new ModifiedNodalAnalysisSolution( { - 0: 0, - 1: 4 - }, [ battery.withCurrentSolution( 2.0 ) ] ); - assert.equal( solution.approxEquals( desiredSolution, assert ), true, 'solution should match' ); - - // same magnitude as battery: positive because current flows from node 1 to 0 - assert.equal( - approxEquals( solution.getCurrentForResistor( resistor ), 2 ), true, 'current through resistor should be 2.0 Amps' - ); -} ); - -QUnit.test( 'test_an_unconnected_resistor_shouldnt_cause_problems', assert => { - const battery = new ModifiedNodalAnalysisCircuitElement( '0', '1', null, 4.0 ); - const resistor1 = new ModifiedNodalAnalysisCircuitElement( '1', '0', null, 4.0 ); - const resistor2 = new ModifiedNodalAnalysisCircuitElement( '2', '3', null, 100 ); - const circuit = new ModifiedNodalAnalysisCircuit( [ battery ], [ resistor1, resistor2 ], [] ); - const desiredSolution = new ModifiedNodalAnalysisSolution( { - 0: 0, - 1: 4, - 2: 0, - 3: 0 - }, [ battery.withCurrentSolution( 1.0 ) ] ); - const solution = circuit.solve(); - assert.equal( solution.approxEquals( desiredSolution, assert ), true, 'solutions should match' ); -} ); - -QUnit.test( 'test_current_source_should_provide_current', assert => { - const currentSource = new ModifiedNodalAnalysisCircuitElement( '0', '1', null, 10 ); - const resistor = new ModifiedNodalAnalysisCircuitElement( '1', '0', null, 4 ); - const circuit = new ModifiedNodalAnalysisCircuit( [], [ resistor ], [ currentSource ] ); - const voltageMap = { - 0: 0, - - // This is negative since traversing across the resistor should yield a negative voltage, see - // http://en.wikipedia.org/wiki/Current_source - 1: -40.0 - }; - const desiredSolution = new ModifiedNodalAnalysisSolution( voltageMap, [] ); - const solution = circuit.solve(); - assert.equal( solution.approxEquals( desiredSolution, assert ), true, 'solutions should match' ); -} ); - -QUnit.test( 'test_current_should_be_reversed_when_voltage_is_reversed', assert => { - const battery = new ModifiedNodalAnalysisCircuitElement( '0', '1', null, -4 ); - const resistor = new ModifiedNodalAnalysisCircuitElement( '1', '0', null, 2 ); - const circuit = new ModifiedNodalAnalysisCircuit( [ battery ], [ resistor ], [] ); - const voltageMap = { - 0: 0, - 1: -4 - }; - - const desiredSolution = new ModifiedNodalAnalysisSolution( voltageMap, [ battery.withCurrentSolution( -2 ) ] ); - const solution = circuit.solve(); - assert.equal( solution.approxEquals( desiredSolution, assert ), true, 'solutions should match' ); -} ); - -QUnit.test( 'test_two_batteries_in_series_should_have_voltage_added', assert => { - const battery1 = new ModifiedNodalAnalysisCircuitElement( '0', '1', null, -4 ); - const battery2 = new ModifiedNodalAnalysisCircuitElement( '1', '2', null, -4 ); - const resistor1 = new ModifiedNodalAnalysisCircuitElement( '2', '0', null, 2.0 ); - const circuit = new ModifiedNodalAnalysisCircuit( [ battery1, battery2 ], [ resistor1 ], [] ); - - const voltageMap = { - 0: 0, - 1: -4, - 2: -8 - }; - const desiredSolution = new ModifiedNodalAnalysisSolution( voltageMap, [ - battery1.withCurrentSolution( -4 ), - battery2.withCurrentSolution( -4 ) - ] ); - const solution = circuit.solve(); - assert.equal( solution.approxEquals( desiredSolution, assert ), true, 'solutions should match' ); -} ); - -QUnit.test( 'test_two_resistors_in_series_should_have_resistance_added', assert => { - const battery = new ModifiedNodalAnalysisCircuitElement( '0', '1', null, 5.0 ); - const resistor1 = new ModifiedNodalAnalysisCircuitElement( '1', '2', null, 10.0 ); - const resistor2 = new ModifiedNodalAnalysisCircuitElement( '2', '0', null, 10.0 ); - const circuit = new ModifiedNodalAnalysisCircuit( [ battery ], [ - resistor1, - resistor2 - ], [] ); - const voltageMap = { - 0: 0, - 1: 5, - 2: 2.5 - }; - const desiredSolution = new ModifiedNodalAnalysisSolution( voltageMap, [ battery.withCurrentSolution( 5 / 20.0 ) ] ); - const solution = circuit.solve(); - assert.equal( solution.approxEquals( desiredSolution, assert ), true, 'solutions should match' ); -} ); - -QUnit.test( 'test_A_resistor_with_one_node_unconnected_shouldnt_cause_problems', assert => { - const battery = new ModifiedNodalAnalysisCircuitElement( '0', '1', null, 4.0 ); - const resistor1 = new ModifiedNodalAnalysisCircuitElement( '1', '0', null, 4.0 ); - const resistor2 = new ModifiedNodalAnalysisCircuitElement( '0', '2', null, 100.0 ); - const circuit = new ModifiedNodalAnalysisCircuit( - [ battery ], - [ resistor1, resistor2 ], [] - ); - const voltageMap = { - 0: 0, - 1: 4, - 2: 0 - }; - const desiredSolution = new ModifiedNodalAnalysisSolution( voltageMap, [ battery.withCurrentSolution( 1.0 ) ] ); - const solution = circuit.solve(); - assert.equal( solution.approxEquals( desiredSolution, assert ), true, 'solutions should match' ); -} ); - -QUnit.test( 'test_an_unconnected_resistor_shouldnt_cause_problems', assert => { - const battery = new ModifiedNodalAnalysisCircuitElement( '0', '1', null, 4.0 ); - const resistor1 = new ModifiedNodalAnalysisCircuitElement( '1', '0', null, 4.0 ); - const resistor2 = new ModifiedNodalAnalysisCircuitElement( '2', '3', null, 100.0 ); - const circuit = new ModifiedNodalAnalysisCircuit( [ battery ], [ - resistor1, - resistor2 - ], [] ); - const voltageMap = { - 0: 0, - 1: 4, 2: 0, 3: 0 - }; - - const desiredSolution = new ModifiedNodalAnalysisSolution( voltageMap, [ battery.withCurrentSolution( 1.0 ) ] ); - const solution = circuit.solve(); - assert.equal( solution.approxEquals( desiredSolution, assert ), true, 'solutions should match' ); -} ); - -QUnit.test( 'test_should_handle_resistors_with_no_resistance', assert => { - const battery = new ModifiedNodalAnalysisCircuitElement( '0', '1', null, 5 ); - const resistor = new ModifiedNodalAnalysisCircuitElement( '2', '0', null, 0 ); - const resistor0 = new ModifiedNodalAnalysisCircuitElement( '1', '2', null, 10 ); - const circuit = new ModifiedNodalAnalysisCircuit( [ battery ], [ - resistor0, - resistor - ], [] ); - const voltageMap = { - 0: 0, - 1: 5, - 2: 0 - }; - const desiredSolution = new ModifiedNodalAnalysisSolution( voltageMap, [ - battery.withCurrentSolution( 5 / 10 ), - resistor.withCurrentSolution( 5 / 10 ) - ] ); - const solution = circuit.solve(); - assert.equal( solution.approxEquals( desiredSolution, assert ), true, 'solutions should match' ); -} ); - -QUnit.test( 'test_resistors_in_parallel_should_have_harmonic_mean_of_resistance', assert => { - const V = 9.0; - const R1 = 5.0; - const R2 = 5.0; - const Req = 1 / ( 1 / R1 + 1 / R2 ); - const battery = new ModifiedNodalAnalysisCircuitElement( '0', '1', null, V ); - const resistor1 = new ModifiedNodalAnalysisCircuitElement( '1', '0', null, R1 ); - const resistor2 = new ModifiedNodalAnalysisCircuitElement( '1', '0', null, R2 ); - const circuit = new ModifiedNodalAnalysisCircuit( [ battery ], [ - resistor1, - resistor2 - ], [] ); - const voltageMap = { 0: 0, 1: V }; - - const desiredSolution = new ModifiedNodalAnalysisSolution( voltageMap, [ battery.withCurrentSolution( V / Req ) ] ); - const solution = circuit.solve(); - assert.equal( solution.approxEquals( desiredSolution, assert ), true, 'solutions should match' ); -} ); \ No newline at end of file +// const approxEquals = ( a: number, b: number ) => Math.abs( a - b ) < 1E-6; Index: js/model/mna/MNAResistor.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/model/mna/MNAResistor.ts b/js/model/mna/MNAResistor.ts new file mode 100644 --- /dev/null (date 1634927359841) +++ b/js/model/mna/MNAResistor.ts (date 1634927359841) @@ -0,0 +1,19 @@ +// Copyright 2021, University of Colorado Boulder + +import ModifiedNodalAnalysisCircuitElement from './ModifiedNodalAnalysisCircuitElement.js'; +import CircuitElement from '../CircuitElement.js'; + +class MNAResistor extends ModifiedNodalAnalysisCircuitElement { + resistanceValue: number; + constructor( nodeId0: string, nodeId1: string, circuitElement: CircuitElement | null, resistanceValue: number, currentSolution: number | null = null ) { + super( nodeId0, nodeId1, circuitElement, currentSolution ); + this.resistanceValue = resistanceValue; + } + + // @public + withCurrentSolution( current: number ) { + return new MNAResistor( this.nodeId0, this.nodeId1, this.circuitElement, this.resistanceValue, current ); + } +} + +export default MNAResistor; \ No newline at end of file Index: js/model/mna/MNABattery.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/model/mna/MNABattery.ts b/js/model/mna/MNABattery.ts new file mode 100644 --- /dev/null (date 1634927359845) +++ b/js/model/mna/MNABattery.ts (date 1634927359845) @@ -0,0 +1,20 @@ +// Copyright 2021, University of Colorado Boulder + +import ModifiedNodalAnalysisCircuitElement from './ModifiedNodalAnalysisCircuitElement.js'; +import CircuitElement from '../CircuitElement.js'; + +class MNABattery extends ModifiedNodalAnalysisCircuitElement { + voltageValue: number; + + constructor( nodeId0: string, nodeId1: string, circuitElement: CircuitElement | null, voltageValue: number, currentSolution: number | null = null ) { + super( nodeId0, nodeId1, circuitElement, currentSolution ); + this.voltageValue = voltageValue; + } + + // @public + withCurrentSolution( current: number ) { + return new MNABattery( this.nodeId0, this.nodeId1, this.circuitElement, this.voltageValue, current ); + } +} + +export default MNABattery; \ No newline at end of file Index: js/model/mna/DynamicCircuitTests.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/model/mna/DynamicCircuitTests.ts b/js/model/mna/DynamicCircuitTests.ts --- a/js/model/mna/DynamicCircuitTests.ts (revision d46d5cc8c04a337fbcf9818df55ccf277a0dfd41) +++ b/js/model/mna/DynamicCircuitTests.ts (date 1634927359855) @@ -15,7 +15,7 @@ import DynamicCircuitResistiveBattery from './DynamicCircuitResistiveBattery.js'; import DynamicCircuitInductor from './DynamicCircuitInductor.js'; import DynamicElementState from './DynamicElementState.js'; -import ModifiedNodalAnalysisCircuitElement from './ModifiedNodalAnalysisCircuitElement.js'; +import MNAResistor from './MNAResistor.js'; const ITERATIONS = 250; QUnit.module( 'DynamicCircuit' ); @@ -23,7 +23,7 @@ const errorThreshold = 1E-2; const testVRCCircuit = ( v: number, r: number, c: number, assert: Assert ) => { - const resistor = new ModifiedNodalAnalysisCircuitElement( '1', '2', null, r ); + const resistor = new MNAResistor( '1', '2', null, r ); const battery = new DynamicCircuitResistiveBattery( '0', '1', v, 0 ); const capacitor = new DynamicCapacitor( new DynamicCircuitCapacitor( '2', '0', c ), @@ -66,7 +66,7 @@ } ); const testVRLCircuit = ( V: number, R: number, L: number, assert: Assert ) => { - const resistor = new ModifiedNodalAnalysisCircuitElement( '1', '2', null, R ); + const resistor = new MNAResistor( '1', '2', null, R ); const battery = new DynamicCircuitResistiveBattery( '0', '1', V, 0 ); const inductor = new DynamicInductor( new DynamicCircuitInductor( '2', '0', L ), new DynamicElementState( V, 0.0 ) ); let circuit = new DynamicCircuit( [ resistor ], [ battery ], [], [ inductor ] ); Index: js/model/mna/DynamicCircuitResistiveBattery.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/model/mna/DynamicCircuitResistiveBattery.ts b/js/model/mna/DynamicCircuitResistiveBattery.ts --- a/js/model/mna/DynamicCircuitResistiveBattery.ts (revision d46d5cc8c04a337fbcf9818df55ccf277a0dfd41) +++ b/js/model/mna/DynamicCircuitResistiveBattery.ts (date 1634929259456) @@ -4,6 +4,7 @@ class DynamicCircuitResistiveBattery extends ModifiedNodalAnalysisCircuitElement { readonly voltage: number; resistance: number; + voltageValue: number; /** * @param {string} node0 @@ -16,10 +17,16 @@ // @public this.voltage = voltage; + this.voltageValue = voltage; // @public this.resistance = resistance; } + + // @public + withCurrentSolution( current: number ) { + return new DynamicCircuitResistiveBattery( this.nodeId0, this.nodeId1, this.voltage,this.resistance); + } } export default DynamicCircuitResistiveBattery; \ No newline at end of file Index: js/model/mna/ModifiedNodalAnalysisCircuitElement.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/model/mna/ModifiedNodalAnalysisCircuitElement.ts b/js/model/mna/ModifiedNodalAnalysisCircuitElement.ts --- a/js/model/mna/ModifiedNodalAnalysisCircuitElement.ts (revision d46d5cc8c04a337fbcf9818df55ccf277a0dfd41) +++ b/js/model/mna/ModifiedNodalAnalysisCircuitElement.ts (date 1634928322276) @@ -15,22 +15,18 @@ readonly nodeId0: string; readonly nodeId1: string; readonly circuitElement: CircuitElement | null; - mnaValue: number; currentSolution: number | null; /** * @param {string} nodeId0 * @param {string} nodeId1 * @param {CircuitElement|null} circuitElement, null during qunit tests - * @param {number} value - resistance for resistors, voltage for battery or current for current source * @param {number|null} [currentSolution] */ - constructor( nodeId0: string, nodeId1: string, circuitElement: CircuitElement | null, value: number, currentSolution: number | null = null ) { + constructor( nodeId0: string, nodeId1: string, circuitElement: CircuitElement | null, currentSolution: number | null = null ) { assert && CCKCUtils.validateNodeIndex( nodeId0 ); assert && CCKCUtils.validateNodeIndex( nodeId1 ); - assert && assert( !isNaN( value ) ); - // @public (read-only) {string} index of the start node this.nodeId0 = nodeId0; @@ -40,24 +36,10 @@ // @public (read-only) {CircuitElement|null} index of the start node this.circuitElement = circuitElement; - // @public (read-only) {number} resistance for resistors, voltage for battery or current for current source - this.mnaValue = value; - // @public {number} supplied by the modified nodal analysis this.currentSolution = currentSolution; } - /** - * Creates a new instance matching this one but with a newly specified currentSolution. - * Used in unit testing. - * @param {number} currentSolution - * @returns {ModifiedNodalAnalysisCircuitElement} - * @public (unit-tests) - */ - withCurrentSolution( currentSolution: number ) { - return new ModifiedNodalAnalysisCircuitElement( this.nodeId0, this.nodeId1, this.circuitElement, this.mnaValue, currentSolution ); - } - /** * Determine if the element contains the given node id * @param {string} nodeId Index: js/model/mna/MNACurrentSource.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/model/mna/MNACurrentSource.ts b/js/model/mna/MNACurrentSource.ts new file mode 100644 --- /dev/null (date 1634928130109) +++ b/js/model/mna/MNACurrentSource.ts (date 1634928130109) @@ -0,0 +1,14 @@ +// // Copyright 2021, University of Colorado Boulder +// +// import ModifiedNodalAnalysisCircuitElement from './ModifiedNodalAnalysisCircuitElement.js'; +// import CircuitElement from '../CircuitElement.js'; +// +// class MNACurrentSource extends ModifiedNodalAnalysisCircuitElement { +// currentValue: number; +// constructor( nodeId0: string, nodeId1: string, circuitElement: CircuitElement | null, currentValue: number, currentSolution: number | null = null ) { +// super( nodeId0, nodeId1, circuitElement, currentSolution ); +// this.currentValue = currentValue; +// } +// } +// +// export default MNACurrentSource; \ No newline at end of file ```

[samreid]

The jitter is being caused by the variation in the dt parameters for the timestep. I discovered this by noticing that there is no jitter when pressing the "step" button. Here are some dt values when running the model in the first comment:

0.018
0.016
0.021
0.013
0.017
0.016
0.018
0.016
0.017
0.015
0.017
0.016
0.018
0.015
0.016
0.018
0.017
0.016
0.017

If I set it to be a steady 1/60 (60fps) then the jitter disappears. I observed that in Java, it is a very steady 0.03 seconds between frames.

[samreid]

I published a dev version here that uses a consistent dt of 1/60.0. That means on a slow computer, it won't keep pace with realtime. But we should evaluate and see if that is an acceptable tradeoff.

https://phet-dev.colorado.edu/html/circuit-construction-kit-ac/1.0.0-dev.38/phet/circuit-construction-kit-ac_en_phet.html

This also includes numerous critical internal fixes from https://github.com/phetsims/circuit-construction-kit-common/issues/764 which were not intended to have any change in observable behavior, but the changes were nontrivial and still need to be verified.

@ariel-phet can you please test? Please check the behavior for this issue, as well as one or more resonance test, and a few "wildcard" tests.

[samreid]

Today @ariel-phet and @arouinfar and I discussed this proposed solution, we think it is OK to run at 1/60 constant frame rate. The values in the sim will be internally consistent, even if they don't match up with wall clock time.

[ariel-phet]

@samreid I have not seen any jitter in this kind of circuit in 1.0.0-dev.43 during testing.

Before closing the issue, we might want QA to check performance of a circuit like this on a Chromebook. Marking for design meeting so a test like this can be scheduled.

[samreid]

I can test on my son's school-issued chromebook, but I'll probably do so with capacitorResistance=1E-4 since we are considering that for https://github.com/phetsims/circuit-construction-kit-common/issues/769#issuecomment-961431451

[samreid]

Testing the resonant circuit in the top comment with dev.45 looks fine on the lenovo 300e chromebook. Despite the graininess of this video the motion seemed smooth and speedy.

Running with ?capacitorResistance=1E-4 seemed about the same. I compared sim time to wall time, and found that 3 cycles had passed in the current chart in 5 seconds of real time. I clocked the same circuit on my Macbook Air running 5 cycles in 5 seconds of realtime.

In https://github.com/phetsims/circuit-construction-kit-common/issues/758#issuecomment-953228045 we discussed that it is OK for the sim to be slower than realtime as long as it is internally consistent. But I thought it would be good to double check that it's OK to be as slow as a ratio of 3:5 on devices like this chromebook. But if we decide that is not ok, I'm not sure how to address it. @arouinfar can you please advise? Close if no more work is necessary for this issue.

[samreid]

@arouinfar and I discussed this today. Since it was smooth and speedy on the chromebook, and the time was internally consistent on the chromebook, this issue seems good to close. It is ok running at 3:5 time.

[samreid]

As discovered in https://github.com/phetsims/chipper/issues/946, there are TODOs in the code referring to this issue. Reopening.

[samreid]

I removed the TODO, closing.