phetsims / circuit-construction-kit-common

"Circuit Construction Kit: Basics" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
10 stars 10 forks source link

Current reading increases as I increase resistance #979

Closed Nancy-Salpepi closed 1 year ago

Nancy-Salpepi commented 1 year ago

Test device MacBook Air M1 chip

Operating System 13.2.1

Browser Safari/chrome

Problem description For https://github.com/phetsims/qa/issues/908, with the real bulb, regular resistor and high voltage battery, as I increase the resistance, the current increases.

Steps to reproduce

  1. Create a circuit with the real bulb, resistor, high voltage battery and inline ammeter (see pic below)
  2. Increase the resistance of the resistor

This is different than published.

Visuals

Screenshot 2023-02-28 at 12 50 13 PM
Troubleshooting information: !!!!! DO NOT EDIT !!!!! Name: ‪Circuit Construction Kit: DC‬ URL: https://phet-dev.colorado.edu/html/circuit-construction-kit-dc/1.3.0-rc.1/phet/circuit-construction-kit-dc_all_phet.html Version: 1.3.0-rc.1 2023-02-23 22:57:25 UTC Features missing: applicationcache, applicationcache, touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/16.3 Safari/605.1.15 Language: en-US Window: 1222x654 Pixel Ratio: 2/1 WebGL: WebGL 1.0 GLSL: WebGL GLSL ES 1.0 (1.0) Vendor: WebKit (WebKit WebGL) Vertex: attribs: 16 varying: 30 uniform: 1024 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 16384x16384 OES_texture_float: true Dependencies JSON: {}
samreid commented 1 year ago

@arouinfar can you please test and comment on which behavior is more physically accurate?

arouinfar commented 1 year ago

Good find @Nancy-Salpepi. My guess is the culprit may be https://github.com/phetsims/circuit-construction-kit-common/issues/967.

@samreid without a doubt the behavior in the published version is more physically accurate.

In rc.1, there is something very strange going on with the way the resistance of the real bulb gets recomputed in response to the resistance changing elsewhere in the circuit. It doesn't make any sense to me that increasing the resistance of the resistor should cause the current to increase, everything else being equal.

Start with this circuit (all components have default values): image

Increase the resistance of the resistor gradually until you reach the max. The current will increase and then decrease, reaching a max of 7.85 A at 46 ohms. At the max resistance of 120 ohms, the current is still higher than when you started, 6.00 A vs. 5.34 A. image image

matthew-blackman commented 1 year ago

I think I found the issue in the physics calculations here. The LTA is first using the REAL_BULB_COLD_RESISTANCE to estimate the voltage across the real bulb, then doing one more iteration using this voltage to find the new resistance of the real bulb, and solving the circuit with this resistance. However, in both of the examples @arouinfar showed above, the voltage across the real bulb would be significantly different than the initial value used by the LTA to calculate its resistance.

One option that I can see here would be to increase the number of iterations used to find the resistance of the real bulb. Each loop should get closer to the actual steady-state value, and we can explore how much this result fluctuates based on the number of iterations. Thoughts @samreid @arouinfar?

Note: I was able to calculate current values consistent with the images above using the following equation from the LTA resistorAdapter.resistance = LightBulb.REAL_BULB_COLD_RESISTANCE + 3 * V / logWithBase( V + 2, 2 );, with V as the voltage across the real bulb if its resistance was REAL_BULB_COLD_RESISTANCE. The circuit behavior is consistent with all LTA calculations as far as I can see.

matthew-blackman commented 1 year ago

I created a patch that appears to solve this issue and get the real bulbs extremely close to their ideal behavior, while still avoiding #967. WebStorm is flagging a no-loop-func TS error, but I think @samreid and I can go over how to get around this.

``` diff Subject: [PATCH] Patch for real bulbs iteration in LTA --- Index: js/model/analysis/LinearTransientAnalysis.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/model/analysis/LinearTransientAnalysis.ts b/js/model/analysis/LinearTransientAnalysis.ts --- a/js/model/analysis/LinearTransientAnalysis.ts (revision 374c935a1bc7d89588d39a2922a58b862ba42c36) +++ b/js/model/analysis/LinearTransientAnalysis.ts (date 1677696585235) @@ -149,35 +149,42 @@ // the resistance of real bulbs is a function of their voltage if ( hasRealBulbs ) { - ltaResistors.forEach( resistorAdapter => { - const circuitElement = resistorMap.get( resistorAdapter )!; - if ( circuitElement instanceof LightBulb && circuitElement.isReal ) { + const numIteractionsToComplete = 5; + let numIterationsCompleted = 0; + + while ( numIterationsCompleted < numIteractionsToComplete ) { + ltaResistors.forEach( resistorAdapter => { + const circuitElement = resistorMap.get( resistorAdapter )!; + if ( circuitElement instanceof LightBulb && circuitElement.isReal ) { - const logWithBase = ( value: number, base: number ) => Math.log( value ) / Math.log( base ); + const logWithBase = ( value: number, base: number ) => Math.log( value ) / Math.log( base ); - const dV = circuitResult.getFinalState().ltaSolution!.getVoltage( resistorAdapter.nodeId0, resistorAdapter.nodeId1 ); - const V = Math.abs( dV ); + const dV = circuitResult.getFinalState().ltaSolution!.getVoltage( resistorAdapter.nodeId0, resistorAdapter.nodeId1 ); + const V = Math.abs( dV ); - const base = 2; + const base = 2; - // I = ln(V) - // V=IR - // V=ln(V)R - // R = V/ln(V) + // I = ln(V) + // V=IR + // V=ln(V)R + // R = V/ln(V) - // Adjust so it looks good in comparison to a standard bulb - const coefficient = 3; + // Adjust so it looks good in comparison to a standard bulb + const coefficient = 3; - // shift by base so at V=0 the log is 1 - resistorAdapter.resistance = LightBulb.REAL_BULB_COLD_RESISTANCE + coefficient * V / logWithBase( V + base, base ); - circuitElement.resistanceProperty.value = resistorAdapter.resistance; - } - } ); + // shift by base so at V=0 the log is 1 + resistorAdapter.resistance = LightBulb.REAL_BULB_COLD_RESISTANCE + coefficient * V / logWithBase( V + base, base ); + circuitElement.resistanceProperty.value = resistorAdapter.resistance; + } + } ); - // If the circuit contains real bulbs, we need to solve the circuit again after calculating their resistance - // to prevent a hysteresis. This ensures that the resistance of the bulbs is consistent with their voltage. - ltaCircuit = new LTACircuit( ltaResistors, ltaBatteries, ltaCapacitors, ltaInductors ); - circuitResult = ltaCircuit.solveWithSubdivisions( TIMESTEP_SUBDIVISIONS, dt ); + // If the circuit contains real bulbs, we need to solve the circuit again after calculating their resistance + // to prevent a hysteresis. This ensures that the resistance of the bulbs is consistent with their voltage. + ltaCircuit = new LTACircuit( ltaResistors, ltaBatteries, ltaCapacitors, ltaInductors ); + circuitResult = ltaCircuit.solveWithSubdivisions( TIMESTEP_SUBDIVISIONS, dt ); + + numIterationsCompleted++; + } } // Apply the solutions from the analysis back to the actual Circuit ```
matthew-blackman commented 1 year ago

Confirmed that the real bulb resistance calculcation approaches the correct value with each iteration. The screenshot below shows the logged resistance of the real bulb through 20 LTA iterations in a single frame:

Screen Shot 2023-03-01 at 2 13 19 PM
matthew-blackman commented 1 year ago

@samreid and I will work on commiting the above patch and send over to @arouinfar for testing in comparison to the current version of the live sim.

matthew-blackman commented 1 year ago

@samreid and I investigated this further and found that the published version of the sim also exhibits the behavior that @Nancy-Salpepi and @arouinfar described above. Commit https://github.com/phetsims/circuit-construction-kit-common/commit/b11aeb34eeec7cf56a714474eb6d261ff6a41fee solves the issue and is ready for review.

matthew-blackman commented 1 year ago

@arouinfar can you confirm that the updated behavior is correct, and also that the published sim exhibits the same issues as described above?

samreid commented 1 year ago

Code review looks good to me, thanks! @arouinfar if everything looks good, please mark as ready-for-cherry-pick.

arouinfar commented 1 year ago

Looks good to me too! I made sure to text with extreme batteries, extreme resistors, and long wires with varying resistivity.

samreid commented 1 year ago

For the QA team, please check the cases in the top comment to see that it behaves like "published".

Nancy-Salpepi commented 1 year ago

This looks good in rc.3! Closing.