Fix Error in Ball-Ball Frictional Inelastic No-Slip Velocity Calculation (Again) #177
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #177 +/- ##
==========================================
+ Coverage 44.22% 44.62% +0.40%
==========================================
Files 127 127
Lines 8554 8587 +33
==========================================
+ Hits 3783 3832 +49
+ Misses 4771 4755 -16
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Needs a test for no-slip case; maybe a gearing english cut shot, which should result in no throw. |
All thanks to David Alciatore and Bob Jewett! |
| @@ -90,7 +90,7 @@ def test_translating_head_on_zero_spin_inelastic( | |||
| cb_f, ob_f = model.resolve(cb_i, ob_i, inplace=False) | |||
|
|
|||
| # Balls should still be moving together in +y direction | |||
| assert np.isclose(cb_f.vel[1], ob_f.vel[1], atol=1e-10) | |||
| assert abs(cb_f.vel[1] - ob_f.vel[1]) < 1e-10 | |||
There was a problem hiding this comment.
I changed this to a manual floating point comparison so that the pytest output is easier to read if it fails.
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "model", [FrictionalInelastic(), FrictionalMathavan(num_iterations=int(1e5))] |
There was a problem hiding this comment.
Mathavan takes this many iterations to meet requirements for no throw and no induced english.
| assert abs(ob_f.avel[2]) < 1e-3, "Gearing english shouldn't cause induced side-spin" | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("model", [FrictionalInelastic()]) |
There was a problem hiding this comment.
This test is pretty specific to FrictionalInelastic. Mathavan doesn't really work for this one even with 10e5 iterations, nor would I expect it to.
There was a problem hiding this comment.
The "gearing english" is a great test. Also super illustrative of Mathavan's high iteration requirements... I think we should consider replacing the Mathavan model with this.
One request: Can you update the docstrings of those two tests?
- The spin is in the -z direction
- A mathematical definition of the gearing english condition would be useful because it's hard to follow from the code alone.
- A description of what
test_low_relative_surface_velocitytests would be helpful
It is a cruel experience to have your PR approved, then you improve the code further with tests, then your reviewer requests more changes 😄
No biggie. I was rushing a little when I added those tests, so I'm glad you caught that stuff. |
I found an algebra mistake in my work. Third time is the charm, I hope.
I did a more thorough evaluation and found that the stun-shot plots matched Dr. Dave's TP A-14 if his friction model is used. Most importantly, the no-slip/slip case intersection happens at the same point.