Unable to step back inelastic collisions looks buggy
Closed this issue · 33 comments
Test device
Lovelace
Operating System
ChromeOS
Browser
Chrome
Problem description
For phetsims/qa#562
If the collisions in the sim are inelastic you are unable to use the step back button. @jonathanolson said on Slack:
// The step-backward button is only enabled when the sim is paused, the elasticity is 100%, and the total elapsed
// time isn't 0. There isn't any support to provide a custom enabledProperty to step-buttons. So, we use a
// a workaround. See phetsims/scenery-phet#606 and
// #66. DerivedProperty never disposed since
// CollisionLabTimeControlNode persists for the lifetime of simulation.
This makes sense, however in practice it comes across as inconsistent behavior. It also is particularly noticeable on the Inelastic screen, where activating the step back button seems to be impossible from what I can tell. I also noticed that if collisions are inelastic, the sim plays, and you change them to elastic, you can use the step back buttons to go back in time in a manner that it didn't behave, if that makes sense (not sure how to describe this.
Not sure what a solution would look like, but I thought it was worth bringing up. Especially on the Inelastic screen.
Visuals
This is an example of the step back button showing a sort of alternate history.
@arouinfar, thoughts on how to proceed? It seems like we'd be gaining energy (which is why I'd guess it might not be enabled). Also should we in general avoid cases of alternate history?
I'm pretty sure the reason the step back button is disabled if the elasticity is less than 100% is because the physics will be inaccurate. We should remove the button from the Inelastic screen because it will never be enabled.
Honestly, I'm okay with the alternative history when stepping back so long as the physics is accurate. Right now, it looks like momentum is not conserved when stepping back. For example:
- Explore 2D, default configuration, Kinetic Energy on, Values on, elasticity 50%
- Press play
- After the first collision, pause the sim and take note of the KE and momentum.
- Set elasticity to 100%
- Step back until the balls collide. The KE is conserved, but the momentum increases.
I'd like more details on the momentum increasing. It seems more like the momentum vectors of each ball change so that the sum of their individual magnitudes is more, while the net momentum's magnitude stays the same in this case?
In the Momenta Diagram, I see the total momentum vector stay the same also.
In principle, the total momenta should stay the same if the collision reversed. However, the kinetic energy should increased if the elasticity is set to anything to less than one.
After using the simulation in class, I concur with @KatieWoe that it makes the simulation difficult to operate.
A typical use of the simulation is to set up two ball to a particular initial states (masses, positions and velocities) and then observe the results from the collision.
If one did not pause the simulation after the first collision, then the balls have undoubtedly collided with the walls, another balls, etc, and it is too late for them to collect any data.
I have seen a few students making that mistake repeatedly. Because there is no step back button, they cannot wind back the clock, and therefore they have to go back to the tedious process of setting the positions and velocities again.
I understand that by rewinding the clock, the kinetic energy becomes unbounded, but one suggestion could be to develop a heuristic argument that once the speed of a ball exceeds a particular threshold say 10m/s, then the rewind button would be disabled. In practice, it would mean that for most cases, it would give the students plenty of rewind "time", but would prevent playback problems associated when the speeds become too high.
As @arouinfar mentioned, the approach described above cannot work for the inelastic screen, so the rewind button should be disabled.
Design meeting: For elasticity > 0, step back works until the "reset point", elasticity = 0 it will be disabled. Remove fully from the inelastic screen.
Thanks for your feedback @veillette!
We think the step back is a valuable control, but it's not particularly useful if the state (e.g. mass, elasticity) has changed since it is no longer representative of events that actually happened in the past. Changing the number of balls, mass, velocity, position, or elasticity saves a new state which can be restored by the reset button under the collision window. We decided to enable the step back button until the user reaches this saved state (so long as elasticity > 0). This way, users can step back until the point where the state was changed and they won't encounter a false history.
This is implemented (and also included the implementation of backward stepping with elasticity, oops). @arouinfar can you give it a try?
The step back behavior looks great in master, but I realized that elasticity is not part of the state, so I opened #190.
In 1.1.0 dev 13, adding or taking away a ball does not seem to restart the clock.
I also noticed that just entering the text box to change mass, position, and velocity resets the clock, even if you didn't make any change. This may be the best option however.
Talked to @arouinfar and I understand the expected behavior a bit better now. I noticed another oddity on how it is handled when a ball is taken away or added. If it was added, pressing the blue reset button will take you back to when the ball was added at 0, even though that point wasn't set at 0 before pressing the blue button. If you take a ball away, the blue button will take you back to the wrong reset position, the one that would be set before a ball was removed. The ball will still be gone.
Another odd feature:
When a change is made with one ball outside the play area due to a lack of reflecting barrier (such as on the first screen), the step back button takes you back to the change, but the blue restart button takes you back to the last state where both balls were there.
When a change is made with one ball outside the play area due to a lack of reflecting barrier (such as on the first screen), the step back button takes you back to the change, but the blue restart button takes you back to the last state where both balls were there.
This is in the code explicitly, there's a check for whether balls are all in the play area.
Should the state be changed on the 4th screen when tick and slip are switched? There's no step back button, so it seems to matter less.
I also notice that it doesn't seem to change when you check the constant size checkbox, which can cause further errors.
I noticed another oddity on how it is handled when a ball is taken away or added. If it was added, pressing the blue reset button will take you back to when the ball was added at 0, even though that point wasn't set at 0 before pressing the blue button. If you take a ball away, the blue button will take you back to the wrong reset position, the one that would be set before a ball was removed. The ball will still be gone.
This is a great find @KatieWoe, and seems inconsistent with how we handle other model-changing parameters (e.g. mass).
Should the state be changed on the 4th screen when tick and slip are switched? There's no step back button, so it seems to matter less.
I don't think it's necessary. I think there was some discussion about this previously, but I couldn't find the issue.
I also notice that it doesn't seem to change when you check the constant size checkbox, which can cause further errors.
@KatieWoe if you haven't already, please open issues for the bugs you've seen related to Constant Size.
@jonathanolson I'd be fine with resetting the timer/overriding the blue reset button when constant size is checked if that's easiest.
Are there any changes recommended in this issue (besides the pulled-out constant size issue?)
The number of balls changing didn't seem to be fully handled #183 (comment).
I believe the above commit should handle that, can you verify?
Looks good in master @jonathanolson
Looks dood on master
Leaving open for now due to #199, unless @jonathanolson thinks they are separate enough and this can be closed.
Actually, I am seeing buggy behavior for slightly inelastic collisions being stepped back, where the rewinding behavior does not match what it was going forward. Recorded on Explore 1D. For phetsims/qa#599
Also reproduced on 2D screen
While attempting to reproduce on 2D screen, the sim froze with this console error building up. @jonathanolson is this a separate issue?
Example of very odd behavior on 2d screen
@jonathanolson seems like the solution might be to go back to disabling the step back button for elasticity =/= 0.
I'm hitting this behavior also randomly.
I believe it's due to numerical imprecision that gets magnified significantly for every collision (and glancing collisions magnify it also).
So, I'm inclined to say we should disable the step-back for inelastic collisions. @arouinfar thoughts?
So, I'm inclined to say we should disable the step-back for inelastic collisions. @arouinfar thoughts?
Sounds good!
Implemented, can you verify?
@jonathanolson so sorry for the delay. Looks good in master!
Did some exploring and this does look ok in rc.3. The step back button is disabled for inelastic collisions and I wasn't able to reproduce this while stepping back elastic collisions so far. I did sometimes see differing behavior when stepping back, mentioned in #205 (comment). Since that was deemed acceptable I think this can be closed. Will reopen if something comes up.
