add Mathavan frictional ball-ball collision model

[jzitelli]

This PR still needs some work I think (I want to complete the test case at least). I'm happy to receive any feedback and make changes.

The Mathavan article is online here: https://billiards.colostate.edu/physics_articles/Mathavan_Sports_2014.pdf

[ekiefl]

Hey @jzitelli thanks for this amazing contribution!

I had checked out your branch prior to 6a28471 and noticed the direction that the object ball is "thrown" due to the cue ball's spin is opposite. This commit fixes that. I also noticed the friction coefficient has been reduced and is more realistic. Nice work.

I haven't timed anything but it's not noticeably slower than the frictionless elastic model so that's great.

I noticed that a head-on collision leads to forward translation of the cue ball post-collision, even with backspin. Given that the balls are of equal mass, I don't think this is expected. I was wondering if you agree, and if you have any idea why this might be happening.

[ekiefl]

I'm going to try and write some general tests for ball-ball collision models that both the frictionless and the frictional models can be passed through. Are you OK if I commit directly to this branch?

[jzitelli]

Hey @jzitelli thanks for this amazing contribution!

I had checked out your branch prior to 6a28471 and noticed the direction that the object ball is "thrown" due to the cue ball's spin is opposite. This commit fixes that. I also noticed the friction coefficient has been reduced and is more realistic. Nice work.

I haven't timed anything but it's not noticeably slower than the frictionless elastic model so that's great.

I noticed that a head-on collision leads to forward translation of the cue ball post-collision, even with backspin. Given that the balls are of equal mass, I don't think this is expected. I was wondering if you agree, and if you have any idea why this might be happening.

Not sure but I think this behaviour is possible with the model because it has some damping (default restitution coefficient parameter used is e=0.89 which the paper finds as the best fit to their measurements), so collisions are not perfectly elastic... you might try setting e=1. Do you have a script to set up this scenario that you can share?

[jzitelli]

I'm going to try and write some general tests for ball-ball collision models that both the frictionless and the frictional models can be passed through. Are you OK if I commit directly to this branch?

sure, i think i have to add you as a collaborator on the forked repo to let you commit?

[ekiefl]

Ok I added the test script tests/physics/resolve/ball_ball/test_ball_ball.py and you can test the forward translation case with pytest --pdb -rA -k test_head_on_zero_spin. If you're unfamiliar with pytest's --pdb option, this will enter the python debugger wherever an error is raised.

There is also a test_head_on_z_spin case you can test out. Do we expect spin transfer to occur in the model?

Play testing the new model wasn't working and I couldn't figure out why. It's because there is a separate definition for the physics engine that's used in the interactive shot taking mode. I've removed that in 62f1c81. For play-testing, I also found it useful to set squirt_throttle to 0 in ~/.config/pooltool/physics/resolver.yaml so there is no cue stick deflection when applying side spin.

I set the default physics engine to use the Mathavan model, and bumped the version. The first time you run the new code you should get this message in your terminal:

* /Users/evan/.config/pooltool/physics/resolver.yaml is has version 2, which is
not up to date with the most current version: 3. It will be replaced with the
default.

you might try setting e=1

Your explanation makes sense to me, but I found this didn't help so it must be something else.

Do you have a script to set up this scenario that you can share?

Besides test_head_on_zero_spin, you can run this script to see the magnitude of the effect:

import pooltool as pt

system = pt.System(
    table=(table := pt.Table.default()),
    cue = pt.Cue.default(),
    balls=(
        pt.Ball.create("cue", xy=(table.w / 2, table.l / 2)),
        pt.Ball.create("obj", xy=(3 * table.w / 4, table.l / 2)),
    ),
)

phi = pt.aim.at_ball(system, "obj", cut=0.0)
system.cue.set_state(V0=3.5, b=-0.3, phi=phi)
pt.simulate(system, inplace=True)
pt.show(system)

sure, i think i have to add you as a collaborator on the forked repo to let you commit?

I think when you opened the PR there was a checkbox that says something like, "I allow the owners to push to my branch".

[ekiefl]

Great catch with 2693c0b. The test_head_on_z_spin test passes now 👍

you might try setting e=1

Your explanation makes sense to me, but I found this didn't help so it must be something else.

I was wrong earlier. Setting e=1 does lead to the test_head_on_zero_spin test passing (I was setting the default value of collide_balls, which was being overwritten by the default value for FrictionalMathavan.solve).

It's interesting that when measuring outgoing velocities, the model fits the experimental data best with e=0.89. While I trust their data (the fit seems very good), I wonder if the reduced outgoing speed can be explained (a) phenomena absent in their model or (b) experimental design. For example, speed must be measured over a window of time, during which the object ball have been slowed by the cloth's friction. Did they fit to a parabolic speed trajectory and with accurate cloth friction? I have no idea, but it's clear to me that 0.89 leads to this unrealistic forward translation of the cue ball during head on collisions. I play pool at a high skill level and the only time you ever see this is when the cue ball is heavier than the other balls. It is an annoying effect and it's typical at bars to have mismatched ball masses. The effect is quite pronounced at 0.89 and I feel inclined to bump it up to something like 0.94. I will do some more play testing.

Besides that, I think this is pretty much ready. Currently there are a few new coefficients this model introduces: ball-ball sliding friction, and the ball-ball restitution coefficient. I think I'm going to add these as attributes of the ball, as I have done for sliding and rolling coefficients of friction. This technically could allow for ball-specific values and open up the door for things like this: #69

[jzitelli]

nice work! I just pushed the finished up verification against the published results, lgtm

[codecov-commenter]

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

[ekiefl]

@jzitelli I merged your work into pooltool. Congratulations and thank you for your very meaningful contribution.

I made some changes before merging, here is what's relevant:

• I formatted the code with ruff, which is the formatter used by pooltool. If you contribute in the future, you can make sure this happens on your end by installing the pre-commit hooks mentioned in these instructions: https://pooltool.readthedocs.io/en/latest/getting_started/install.html#from-source
• I added docstrings and types to the code. I formatted things so that the ball-ball API reference looks nice: https://pooltool.readthedocs.io/en/latest/autoapi/pooltool/physics/resolve/ball_ball/index.html
• I added the number of iterations of the numerical solver as a parameter to FrictionalMathavan

This is a huge improvement to pooltool's realism and I can't overemphasize my appreciation! Thanks again and I hope you consider contributing more.