phetsims / capacitor-lab-basics

"Capacitor Lab: Basics" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
6 stars 4 forks source link

Charge in top plate changes inappropriately #256

Closed KatieWoe closed 3 years ago

KatieWoe commented 5 years ago

Test device: Dell and Jordan Operating System: Win 10 and Mac 10.14 Browser: chrome and safari Problem description: For https://github.com/phetsims/QA/issues/306, may be connected to https://github.com/phetsims/capacitor-lab-basics/issues/234. When the sim is paused and discharging, moving the capacitors seems to make the amount of charge change. This should not be possible, and steping forward one step causes the charge to jump to the correct amount. Steps to reproduce:

  1. Charge the plates on the lightbulb screen
  2. Turn on top plate charge bar graph
  3. Pause the sim
  4. Move the wires to connect to the bulb
  5. Step forward and observe charge behavior
  6. Move plates closer together or further away (while still paused)
  7. Observe new charge bar
  8. Step forward and observe

Screenshots: changingchargeamount

Troubleshooting information (do not edit):

Name: ‪Capacitor Lab: Basics‬ URL: https://phet-dev.colorado.edu/html/capacitor-lab-basics/1.7.0-dev.7/phet/capacitor-lab-basics_all_phet.html Version: 1.7.0-dev.7 2019-04-10 22:42:01 UTC Features missing: touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/73.0.3683.103 Safari/537.36 Language: en-US Window: 1536x722 Pixel Ratio: 2.5/1 WebGL: WebGL 1.0 (OpenGL ES 2.0 Chromium) GLSL: WebGL GLSL ES 1.0 (OpenGL ES GLSL ES 1.0 Chromium) Vendor: WebKit (WebKit WebGL) Vertex: attribs: 16 varying: 30 uniform: 4096 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 16384x16384 OES_texture_float: true Dependencies JSON: {}
arouinfar commented 5 years ago

Nice find @KatieWoe!

Since the published version doesn't have a pause button, it is hard to tell if this bug exists there, but it doesn't look like it.

Definitely looks like charge isn't being conserved here @Denz1994. Moving the plates while the sim is paused should never affect the amount of charge. While discharging, the charge on the plates would exponentially decay.

Denz1994 commented 5 years ago

@arouinfar I believe this issue persisted in the published version. You see the change when following the steps without pausing, but it is a lot less apparent.

The top plate charge only updates when the capacitor is connected to the battery. @arouinfar Please double check that this behavior is correct in master.

arouinfar commented 5 years ago

@Denz1994 master still looks off to me.

When the capacitor is disconnected from the battery, changing its capacitance results in a change in the voltage (Q is conserved and Q = CV). Here, decreasing C results in proportionally increased V.

ConservedQ

Similarly, when connected to the bulb, changing C should create an instantaneous change in V. Here, decreasing C from 1.77 pF to 0.35 pF should have increased the voltage from 1.5 V to 7.5 V, resulting in a larger halo. Instead, the voltage remains 1.5 V, and the charge is instead reduced. ConservedQ2

If you run into any questions, or would like to discuss over Zoom, please let me know.

Denz1994 commented 5 years ago

this.capacitor.discharge( ) was is responsible for updating the inverse relationship between capacitance and voltage, when the light bulb was connected. This function was being called during step() so when the sim was paused this wasn't being handled.

The capacitor now discharges when the plate geometry changes which should maintain conservation of charge. @arouinfar could you confirm in master?

arouinfar commented 5 years ago

Looks great @Denz1994!

KatieWoe commented 5 years ago

May be connected to https://github.com/phetsims/capacitor-lab-basics/issues/259. As such, will leave up to @Denz1994 on whether this is fixed/whether to close

Denz1994 commented 5 years ago

The changes in https://github.com/phetsims/capacitor-lab-basics/issues/259 should not have affected this, though closely related. This issue should be verified by QA to ensure it is resolved.

KatieWoe commented 5 years ago

This seems to occur under more specific circumstances now. A bit harder to reproduce now.

  1. Go to the light bulb screen
  2. Add charge to the plates
  3. Select the check box for reading the top plate charge (easier to see change)
  4. While battery is still connected change either the area or distance of the plates
  5. Pause the sim
  6. While paused, switch to the light bulb connections
  7. While still paused, move the plates closer/further away or bigger/smaller
  8. The instant you do that the amount of charge will change. It will stay at the same charge with further movement

Edit: It seems that just adjusting the charge with the battery while paused will cause this.

Removing the ready-for qa label. specificcircumstances

KatieWoe commented 5 years ago

https://github.com/phetsims/QA/issues/321

arouinfar commented 5 years ago

Great find @KatieWoe.

@Denz1994 it looks like the plate charge isn't getting properly stored/updated on pause.

  1. On startup, open Light Bulb screen
  2. Turn on Top Plate Charge graph, then charge capacitor. Make a note of the Top Plate Charge value.
  3. Pause sim, then change capacitance (e.g. decrease separation). This results in a change in Top Plate Charge, as it should.
  4. Move switch to light bulb.
  5. Change capacitance (e.g. increase separation). The Top Plate Charge should match the value in step 3, but instead reverts to the Top Plate Charge in step 2.
Denz1994 commented 5 years ago

After much digging, it turns out that capacitor.discharge() should not be used when dragging the plate area and separation arrows. It causes the capacitor to leak voltage and produced a side effect where the charges could be built up to indefinitely when swapping to an open circuit.

The voltage is now explicitly set using Q=V/C when dragged. This prevents charges from being lost and the voltage drops or increases accordingly.

KatieWoe commented 5 years ago

Not fixed. Now, you must pause the sim, switch to light bulb, move the plates, and then step through while paused. https://github.com/phetsims/QA/issues/349 stillchargejumps

Edit: Also happens if you step through first, then adjust the plates. morebehavior

etwa4650 commented 5 years ago

https://drive.google.com/open?id=1-8shThGK3r3NfECoiMY9IpNRyc2rRm9q

Some more strange behavior if you pause the sim after switching to the lightbulb, and then move plates closer while paused. Top plate charge will jump back up to what it was before connecting to the lightbulb (0.27 in this video).

KatieWoe commented 5 years ago

One more oddity here. When making the plate larger and smaller or changing distance while discharging the charge jumps up and down. But I've only seen it so far on IE Win 7. Wondering if this is due to performance or something with IE. Edit: Went back to check other platforms. Seems to happen on IE, Edge, and Safari.

KatieWoe commented 4 years ago

In master when doing ES6 Testing. Moving the plates when not connected to the battery or bulb makes all the charge go away when on the light bulb screen. If the plates are close enough together charges come back, but behave as if the plates are connected to the battery. comesback

Denz1994 commented 4 years ago

Instead of setting the top plate charge when the capacitance changes, the charges are discharged. This bug was introduced when trying to explicitly set the capacitor charge when the plate geometry or separation changes. I've reviewed this briefly with @arouinfar but she will review further in this RC version.

arouinfar commented 4 years ago

@Denz1994 looks good in rc.4

loganbraywork commented 4 years ago

This still seems to be broken as of https://github.com/phetsims/QA/issues/565. By following the steps laid out in the comment above.

This seems to occur under more specific circumstances now. A bit harder to reproduce now.

  1. Go to the light bulb screen
  2. Add charge to the plates
  3. Select the check box for reading the top plate charge (easier to see change)
  4. While battery is still connected change either the area or distance of the plates
  5. Pause the sim
  6. While paused, switch to the light bulb connections
  7. While still paused, move the plates closer/further away or bigger/smaller
  8. The instant you do that the amount of charge will change. It will stay at the same charge with further movement

Edit: It seems that just adjusting the charge with the battery while paused will cause this.

Removing the ready-for qa label. specificcircumstances

Currently. using the switch still causes a jump in top plate energy if the sim was paused while the plates were adjusted. 2020-10-16CapacitorlabbasicsChargeinappropriately

Denz1994 commented 4 years ago

A patch was made to remove explicitly setting the discharge parameters when the geometry changes. This should be handled by discharging. This is more apparent when we can pause because discharging happens during the step. Can you verify this is fixed in this version @loganbraywork?

KatieWoe commented 4 years ago

It now seems to be back to the original behavior where it changes when the plate is changed consistently. In fact, it seems worse since it doesn't jump back to correct behavior when you play/step forward. I also notice the play and step forward buttons are behind the light from the bulb now, making them almost impossible to use. backtobuggy

arouinfar commented 4 years ago

Charge is definitely not being conserved in master. For example:

  1. Turn on Top Plate Charge
  2. Charge capacitor (voltage irrelevant)
  3. Disconnect capacitor, and take note of top plate charge.
  4. Change plate separation or area, and the charge changes. This should not happen.
KatieWoe commented 4 years ago

I'm not always seeing as much as I saw in https://github.com/phetsims/capacitor-lab-basics/issues/256#issuecomment-712965729, and have some trouble reproducing to that degree for some reason. But even so I get a consistent one time jump when I start to move the plates.

Denz1994 commented 3 years ago

The discharge model was simplified in the above commit. Also, the previousCapacitance was being changed as a side effect of discharging but used elsewhere (see updateDischargeParameters).

The steps in https://github.com/phetsims/capacitor-lab-basics/issues/256#issuecomment-710621089 should be verified by QA.

KatieWoe commented 3 years ago

Things are looking good in rc.8 so far. Closing for now, but will keep an eye out and reopen if something reappears.

CliffordH123 commented 3 years ago

This has come back for https://github.com/phetsims/QA/issues/616 plateChargeChange Steps:

  1. Charge plates
  2. Pause Sim.
  3. Switch plates to bulb
  4. Do not un-pause
  5. Change Plate distance
  6. Observe top plate charge graph change
arouinfar commented 3 years ago

Good find @CliffordH123. Looks like yet another regression @jonathanolson, so I'm wondering if we should pause/cancel this dev test.

jonathanolson commented 3 years ago

@KatieWoe I believe this is fixed in master, can you verify?

KatieWoe commented 3 years ago

Adding this to issues to verify on QA board

brooklynlash commented 3 years ago

Looks fixed on master, Win10 Chrome.

loganbraywork commented 3 years ago

This appears fixed as of https://github.com/phetsims/QA/issues/640 on Win 10 Chrome.