phetsims / unit-rates

"Unit Rates" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
0 stars 2 forks source link

Should (dt) be capped? #193

Closed Denz1994 closed 7 years ago

Denz1994 commented 7 years ago

Sim skips ahead when switching tabs.

Steps to reproduce:

  1. Start sim
  2. Enter shopping screen.
  3. Drag item bag to the edge of screen bounds.
  4. Release item bag and before bag reaches its destination, switch tabs in the browser.
  5. Wait a few seconds.
  6. Switch back to sim. At this point, the bag will jump to its destination.

*Note: A similar approach can be done for the cars in the racing lab screen.

Is the timestep dt capped appropriately? Try switching applications or browser tabs, then switch back. Did the model take one big/long/awkward step forward? If so, dt may need to be capped. Example from faradays-law.FaradaysLawModel: // Cap large dt values, which can occur when the tab containing // the sim had been hidden and then re-shown dt = Math.min( 0.1, dt );

Denz1994 commented 7 years ago

159

pixelzoom commented 7 years ago

There is no large dt in this sim. What you're seeing is that the animation pass when switching screen, and continues where it left off when you return to the screen (see https://github.com/phetsims/unit-rates/issues/155).

To convince yourself that there is no large dt:

  1. Add console.log( 'dt=' + dt ); after line 104 in URMovable.
  2. Run the sim with ?animationSpeed=100 to slow down animation.
  3. Got to Shopping screen
  4. Drag a bag to upper corner of screen
  5. Switch to Shopping Lab screen
  6. Note the last dt= value printed to the console.
  7. Clear the console.
  8. Wait a minute or so
  9. Switch back to Shopping screen
  10. Note the first dt= value printed to the console after switching back. It will not be a large value.
pixelzoom commented 7 years ago

For example, here's what I see:

...
dt=0.035
dt=0.021
dt=0.013
dt=0.016
dt=0.013
dt=0.017
(switch to Shopping Lab screen, clear console, wait a minute, switch back to Shopping screen)
dt=0.017
dt=0.072
dt=0.036
dt=0.032
...
pixelzoom commented 7 years ago

Similar for cars in the racing screen. There's no big dt jump, they simply resume animation when you switch back to the screen.

Denz1994 commented 7 years ago

I agree with your explanation above and was able to reproduce similar dt values with the steps given.

If I insert a console.log( 'dt=' + dt ); after 155 in racingLab/model/RaceCar.js. I get these results using the method described above, but instead of switching screens, I switch browser tabs. Also switching browser tabs doesn't make the animation continue from where it was left off at the time of the switch.

dt = 0.02 dt = 0.02 dt = 0.017 dt = 0.017 dt = 0.018 dt = 0.018 (tabs switched) dt = 5.602 dt = 5.602

Is this a different issue from dt values as described in the code review doc?

pixelzoom commented 7 years ago

Ah... switching browser tabs, thanks for clarifying. Indeed, it looks like that's not being handled properly. I'll take care of it.

pixelzoom commented 7 years ago

dt is capped in the above commit. Looking at other sims (ProportionModel), it's best to cap it in the step function of the top-level of the model. In our case, that's ShoppingModel and RacingLabModel. (ShoppingLabModel is a subtype of ShoppingModel, so inherits step and the dt cap.)

@Denz1994 can you please verify in master?

pixelzoom commented 7 years ago

Btw... I chose the cap value of 0.1 by inspection of typical dt values. They were generally in the range of 0.2, but sometimes as high as 0.75. A value of 0.1 is low enough not to dampen legitimate values, but high enough to prevent any huge jumps when switching back to the sim's tab.

Denz1994 commented 7 years ago

Understood. I tested the dt values with the repeated methodology described above and received these values.

dt = 0.017 dt = 0.017 dt = 0.017 dt = 0.017 dt = 0.017 dt = 0.017 dt = 0.017 dt = 0.017 dt = 0.017 dt = 0.025 dt = 0.025

Similar values are also recovered in the racing lab scene. Also, tab switching doesn't interfere with animation of items/bags. Thanks for the update. Closing.