phetsims / forces-and-motion-basics

"Forces and Motion: Basics" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/forces-and-motion-basics
GNU General Public License v3.0
7 stars 10 forks source link

Velocity readout does not match instance proxy #198

Closed jessegreenberg closed 8 years ago

jessegreenberg commented 8 years ago

While working on https://github.com/phetsims/forces-and-motion-basics/issues/197 I noticed that the velocity readout is exactly twice the value in the instance proxy. capture

I remember doing something with the velocity readout to work around an issue with the time step. Searching for issue...

jessegreenberg commented 8 years ago

Here is the issue ---> #192.

jessegreenberg commented 8 years ago

Here is the comment in the code:

// the readout is the property value multiplied by two because the time step is dt in MotionModel.step is    
// reduced by a factor of 2 so that the animation matches the behavior of the Java

The model needs to match the view for use with the instance proxy.

jessegreenberg commented 8 years ago

In MotionModel.step()

//There are more than 2x as many frames on html as we were getting on Java, so have to decrease the dt to compensate
dt = dt / 2.0;

Position and velocity both depend on dt which is why they are half of what they should be when checking with a stop watch and the instance proxy. I think the best way to fix this is to remove dt = dt / 2 in step, and then modify PusherNode and MovingBackgroundNode. The view then handles the animation such that it matches the Java sim.

@samreid @ariel-phet do you agree with this? Should this be done with the maintenance redeploy for #197? The fix shouldn't take much time, but it would be best if QA could test before the maintenance release since we are changing a few things.

samreid commented 8 years ago

Yes, it seems good to investigate the solution you described. I'll leave it to @ariel-phet to comment on priority and scheduling.

ariel-phet commented 8 years ago

@jessegreenberg - if I recall we also did some tweaking so that when acceleration is calculated the speedometer actually rises at the proper rate, so lets also make sure to preserve that behavior.

But your plan sounds good.

jessegreenberg commented 8 years ago

Thanks @samreid and @ariel-phet. I think https://github.com/phetsims/forces-and-motion-basics/issues/198#issuecomment-246005610 was the only tweaking we did, but I will be sure to double check and make sure that the model is correct after these changes.

jessegreenberg commented 8 years ago

Done in the commit above. Changes were done as described in https://github.com/phetsims/forces-and-motion-basics/issues/198#issuecomment-246388436. I just checked in an instance proxy that velocity values match the view. I also verified the model with a stop watch a few times, and values were correct. For instance, a system with:

Net Force = 406 N, Mass = 50 kg, a = 8.1 m/(s*s) was timed for 4.63 seconds. Velocity was 38 m / s and position was 87.3 m (discovered in instance proxy). Velocity should be 37.59 m/s and distance 87.34 m for t=4.63 seconds, so that seems good to me.

phet-steele commented 8 years ago

Values matched in the instance proxy.