phetsims / collision-lab

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

Balls merge at slow/high speeds with inelastic collisions #184

Closed DevonQui closed 3 years ago

DevonQui commented 4 years ago

Test device MacBook Pro

Operating System Catalina 10.15.1

Browser Safari

Problem description This is for https://github.com/phetsims/QA/issues/562 At both slow and high speeds with inelastic collisions, some of the balls will merge. In addition, after changing the initial state of the simulation with the balls still merged, the reset button (not reset-all button) will not "un-merge" the balls. However, the balls do become un-merged once the reset-all button is pressed.

Steps to reproduce

  1. Open the simulation
  2. Click on the Explore 1D section of the simulation
  3. Start off with two balls and start the simulation
  4. Add the additional balls to get a total of four balls and allow them to interact
  5. Wait for some of the balls to get going in the same direction and then turn on inelastic collisions to the max
  6. Wait for the balls to all collide and if done correctly observe how 1 or more of the balls merge

NOTE: If you need help reproducing the bug, feel free to message me on Slack

Visuals https://drive.google.com/file/d/1fhDVRmawgrr1xX3k6on_TyQMlsxueTCB/view?usp=sharing

Troubleshooting information: !!!!! DO NOT EDIT !!!!! Name: ‪Collision Lab‬ URL: https://phet-dev.colorado.edu/html/collision-lab/1.1.0-dev.10/phet/collision-lab_all_phet.html Version: 1.1.0-dev.10 2020-10-15 01:57:00 UTC Features missing: touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_1) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/13.0.3 Safari/605.1.15 Language: en-US Window: 1260x720 Pixel Ratio: 2/1 WebGL: WebGL 1.0 GLSL: WebGL GLSL ES 1.0 (1.20) Vendor: WebKit (WebKit WebGL) Vertex: attribs: 16 varying: 15 uniform: 1024 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 16) Max viewport: 8192x8192 OES_texture_float: true Dependencies JSON: {}

jonathanolson commented 4 years ago

Reproducing ball states:

  new BallState( new Vector2( -1, 0 ), new Vector2( 1.22, 0 ), 0.5 ),
  new BallState( new Vector2( -0.35, 0 ), new Vector2( 0.45, 0 ), 1.5 ),
  new BallState( new Vector2( 0.2, 0 ), new Vector2( 0.14, 0 ), 1 ),
  new BallState( new Vector2( 0.9, 0 ), new Vector2( 1.10, 0 ), 1 )
jonathanolson commented 3 years ago

There are a whole lot of numeric precision issues here, where what is effectively happening is when the first 3 collide, it goes through a number of iterations that set the velocity of 2 balls at a time to be the same. This would converge at the correct value, but we cut off the number of iterations. The ball on the left ends up with a slightly higher velocity, and thus passes INTO the other ball imperceptibly as the 3 go to the right.

I've sunk a lot of time into trying to figure out a good way to solve this that works with the existing code and setup, and I have some general ideas but they would be fairly intensive to implement and maintain.

@arouinfar and @ariel-phet, is the 100% inelastic setup on the Explore 1D screen important, AND is this buggy case important enough to solve for the first publication?

ariel-phet commented 3 years ago

@jonathanolson I played around for awhile and I can reproduce this buggy situation pretty easily. I found a more "minimum reproducible case:

  1. Go to Explore 1d screen
  2. Set balls to 3
  3. Set the elasticity slider to 0%
  4. Set the velocity of ball one to be to the right and reasonably sized (0.5 to 2 m/s appears to work)
  5. Set ball 2 and 3 to zero velocity
  6. Run the experiment (play button) - the balls reliably merge

I would like to discuss with you as I have a few thoughts. The 100% inelastic maybe not be necessary on this screen (but it would be nice). If there is not a simple solution, it would probably be reasonable to have this screen act like the explore 2D screen and have elasticity limited to 5%.

jonathanolson commented 3 years ago

Discussed a solution where on this screen ONLY, it will inspect on collisions to determine all "connected" balls, and will use the combined momentum to determine the group velocity.

jonathanolson commented 3 years ago

I believe this should be patched with the above commit. @ariel-phet can you give it a try?

ariel-phet commented 3 years ago

@jonathanolson this solution seems to be working quite well! Closing

phet-steele commented 3 years ago

I'm still seeing this behaviour in 1.1.0-rc.3 on the Exlpore 1D screen under the following conditions:

  1. 5 balls
  2. Elasticity at 5% (5% is important!)
  3. Then these initial values:

image

Constant Size didn't seem to matter. When running this experiment, Balls 3 & 5 overlap for me.

Seen on Win 10 Chrome. For phetsims/QA/issues/615.

Troubleshooting Information URL: https://phet-dev.colorado.edu/html/collision-lab/1.1.0-rc.3/phet/collision-lab_all_phet.html Version: 1.1.0-rc.3 2021-02-24 17:20:08 UTC Features missing: applicationcache, applicationcache, touch User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/88.0.4324.182 Safari/537.36 Language: en-US Window: 1920x937 Pixel Ratio: 1/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: 32767x32767 OES_texture_float: true
kathy-phet commented 3 years ago

https://user-images.githubusercontent.com/5096804/109367263-dd46fb00-7852-11eb-8c5e-5d3d6a8b4e7a.mp4

kathy-phet commented 3 years ago

@arouinfar and I looked at this, and yes, hoping this can be fixed @jonathanolson. Do you have ideas?

Nice catch Steele!

KatieWoe commented 3 years ago

Great catch!

KatieWoe commented 3 years ago

I encountered this again in the wild with a different set up, and moving the balls around a bit still allowed it to occur, so it may be more general and/or reproducible.

https://user-images.githubusercontent.com/41024075/109569319-7f5e2180-7aa5-11eb-8b20-d7abc2c4d02e.MOV

KatieWoe commented 3 years ago

Saw an instance on 2D screen

https://user-images.githubusercontent.com/41024075/109574696-a882b000-7aad-11eb-8812-69d94b4384b8.MOV

jonathanolson commented 3 years ago

I think I've adjusted the thresholds to a sufficient level in both master and the 1.1 branch. It's tricky trying to find the right level, or other bad behavior could occur.

@KatieWoe and @phet-steele, are you able to test in master to see if this seems resolved? If so, I'll probably need to ask for some more physics testing, as this could potentially trigger issues with more parallel-moving balls.

jonathanolson commented 3 years ago

Once this is verified in master, I'll create an RC for the spot-check.

phet-steele commented 3 years ago

The above buggy behaviors looked fixed on master, but beyond that I am in no way comfortable signing off on this when it may need "more physics testing". I did mess around with it for about 1.25 hours now, and I didn't see any merging balls.

jonathanolson commented 3 years ago

Sounds best then if I create an RC for spot-checking (both this, and ensuring that it doesn't cause other physics issues?)

KatieWoe commented 3 years ago

Yeah, a strong focus on physics for this one is a good idea. We've been keeping an eye on it, but as this shows there are so many set ups that something can still slip by.

jonathanolson commented 3 years ago

Sounds good. RC up at https://github.com/phetsims/QA/issues/620.

KatieWoe commented 3 years ago

Does seem fixed in rc.4.