Skip to content

331 feat adding spearman pitch control#362

Open
twar-TuVie wants to merge 2 commits into
Alek050:developfrom
twar-TuVie:331-feat-adding_spearman_pitch_control
Open

331 feat adding spearman pitch control#362
twar-TuVie wants to merge 2 commits into
Alek050:developfrom
twar-TuVie:331-feat-adding_spearman_pitch_control

Conversation

@twar-TuVie
Copy link
Copy Markdown

First implementation of pitch control model after Spearman(2017)

This is the first implementation of the pitch control suggested by Spearman(2017). It should work, but there are still a lot of things to be done, like mathematical refinement and also making the code more efficient
The calculation of the pitch control was changed to be more true to the original intent of spearman.
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 1, 2026

Codecov Report

❌ Patch coverage is 13.04348% with 100 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.41%. Comparing base (e4ce2b3) to head (3156ba4).
⚠️ Report is 79 commits behind head on develop.

Files with missing lines Patch % Lines
databallpy/features/pitch_control_spearman.py 11.11% 88 Missing ⚠️
databallpy/schemas/tracking_data.py 25.00% 12 Missing ⚠️

❌ Your patch check has failed because the patch coverage (13.04%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #362      +/-   ##
===========================================
- Coverage    99.22%   97.41%   -1.82%     
===========================================
  Files           49       67      +18     
  Lines         3736     5291    +1555     
===========================================
+ Hits          3707     5154    +1447     
- Misses          29      137     +108     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Alek050
Copy link
Copy Markdown
Owner

Alek050 commented Apr 20, 2026

Hi @twar-TuVie,

Thanks for opening the first implementation of the pitch control!

I have a few points that relate to performance for now:

  1. build_ndarray and prepare_data together call multiple regex matches per frame. Regex is relatively slow. I would suggest pre-building arrays per frame in tracking_data.get_spearman_pitch_control instead. This way, you can also use vectorized computation to do it over all frames at ones instead of re-computing for every frame.
  2. in get_spearman_pitch_control_single_frame try to pre-create empty np arrays using np.empty since appending over large lists will become slow. Also change default n_x_bins and n_y_bins to (32, 24) or something, that will be good enough for 95% of use cases and make it 4 times quicker in computation.

These are just some first observations. A few questions to make this more productive:

  1. Is there anything else you want feedback on?
  2. I will go more in depth in optimization of the code later on, did you try a line-profiler yet to see what the current bottlenecks are?
  3. Can you point out which parameters were not present in the original spearman paper? (e.g. max_velociyt and reaction time?)

The CI test are failing because the code is not formatted the way it should be. runn the following commands to fix the linters:

poetry run ruff format tests/ databallpy/
poetry run ruff check --fix tests/ databallpy/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants