mberk / shin

Python implementation of Shin's method for calculating implied probabilities from bookmaker odds
MIT License
74 stars 9 forks source link

feature: implied odds calculator #6

Open peterschutt opened 5 months ago

peterschutt commented 5 months ago

A natural counterpart to probability inference would be odds inference, i.e., output a set of prices for a given set of probabilities (that sum to 1) for a target overround/margin, where the sum of the inverse output prices would = 1 + target overround/margin.

def test_calculate_implied_odds() -> None:
    odds = [2.6, 2.4, 4.3]
    margin = sum([1 / o for o in odds]) - 1
    implied_probabilities = shin.calculate_implied_probabilities(odds)
    res = shin.calculate_implied_odds(implied_probabilities, margin=margin)
    assert pytest.approx(res) == odds

Would you be interested in including something like this in the library?

R's implied package does this, and I've put together a very raw python implementation based from that which you can see diff'd against the typing branch here: https://github.com/peterschutt/shin/pull/1/files.

mberk commented 5 months ago

I think it's a useful addition. A couple of comments:

  1. I would call the parameter overround rather than margin as I think the term "margin" is less well defined
  2. I'd rather do the optimisation in rust
peterschutt commented 5 months ago

I think it's a useful addition. A couple of comments:

1. I would call the parameter overround rather than margin as I think the term "margin" is less well defined

2. I'd rather do the optimisation in rust

Yep, all that makes sense to me.

I'm not particularly comfortable in rust yet, but I'll have a crack at it for the experience. Would you like to keep a python implementation available too?

mberk commented 5 months ago

Your question is actually more nuanced than I immediately thought. My immediate reaction was that we should have a Python optimiser as well so that we match the situation with calculate_implied_probabilities

However, for calculate_implied_probabilities the optimiser in both cases is the one described in

B. Jullien and B. Salanié, "Measuring the Incidence of Insider Trading: A Comment on Shin". The Economic Journal, 1994, 104(427), pp. 1418–1419

as opposed to a general purpose one

While I completely understand why you've done it, implementing a general purpose solver in the package as you have done in your branch doesn't sit entirely well with me but then neither does introducing a heavy weight dependency to the package such as scipy

Reflecting on the history of the package development, I think that probably the only reason the Python optimiser implementation remains is because of backwards compatibility/uncertainty over the newer Rust one. Given that I've already introduced Rust code to the package, I think it makes sense that for entirely new functionality there's no need to replicate code in both Python and Rust and we should just have the optimiser implemented in Rust.

I'd also be very happy to introduce a dependency at the Rust level on a crate that provides a general purpose optimiser although I'm not sure if there is a canonical choice here. Perhaps https://github.com/argmin-rs/argmin. Note that for reasons I cannot currently recall, in one of my other packages I simply ported scipy.optimize._optimize._minimize_scalar_bounded by hand: https://github.com/mberk/kelly/blob/26164e45b102e7decead3f67df10fb119ee5bc61/src/lib.rs#L5-L146 (if I had to say, I think there might have been issues finding a Rust optimiser that was truly bounded)

peterschutt commented 4 months ago

Note that for reasons I cannot currently recall, in one of my other packages I simply ported scipy.optimize._optimize._minimize_scalar_bounded by hand: https://github.com/mberk/kelly/blob/26164e45b102e7decead3f67df10fb119ee5bc61/src/lib.rs#L5-L146 (if I had to say, I think there might have been issues finding a Rust optimiser that was truly bounded)

I think easiest would be to copy that code straight in here then, would that suit? Where are you heading with the # TODO comments in that func?

mberk commented 4 months ago

Copying it in would be fine - certainly I don't personally currently have the capacity to pick an off the shelf alternative or confirm that the port is required if none of them provide exactly the required functionality

I haven't made any progress with those TODOs but I don't think they'll create any problems in this case

peterschutt commented 4 months ago

In the notes for the public function that wraps that scipy minimization func (ref):

Notes
-----
Finds a local minimizer of the scalar function `func` in the
interval x1 < xopt < x2 using Brent's method. (See `brent`
for auto-bracketing.)

... and the argmin crate has a Brent implementation, so I've gone ahead and used that.

I'll be spending more time on it as my rust is pretty raw, but feel free to comment on it at any point if you have the time (https://github.com/peterschutt/shin/pull/1/files) - but also no pressure.