-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Some scripts for plotting graphs to help understand how kelly bet behaves #399
Conversation
Co-authored-by: Peter Jung <peter@jung.ninja>
WalkthroughThe pull request introduces significant enhancements to the betting strategies within the prediction market agent tooling. Key changes include the implementation of two new functions for calculating bets based on the Kelly criterion, improvements to the Changes
Possibly related issues
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range and nitpick comments (2)
scripts/mm_vs_kelly_plot.py (2)
27-103
: LGTM, with nitpicks.The main logic for calculating and plotting bets is approved.
Nitpicks:
- Line 47: Avoid equality comparisons to
True
; useif market_moving_bet.direction:
for truth checks.- Line 61: Avoid equality comparisons to
True
; useif kelly_bet.direction:
for truth checks.Apply this diff to address the nitpicks:
- if market_moving_bet.direction == True + if market_moving_bet.direction - kelly_bet_signed = ( - kelly_bet.size if kelly_bet.direction == True else -kelly_bet.size - ) + kelly_bet_signed = kelly_bet.size if kelly_bet.direction else -kelly_bet.sizeTools
Ruff
47-47: Avoid equality comparisons to
True
; useif market_moving_bet.direction:
for truth checksReplace with
market_moving_bet.direction
(E712)
61-61: Avoid equality comparisons to
True
; useif kelly_bet.direction:
for truth checksReplace with
kelly_bet.direction
(E712)
104-111
: LGTM, with a nitpick.The plot configuration code is approved.
Nitpick:
- Line 104: Remove the extraneous
f
prefix from the f-string as it doesn't contain any placeholders.Apply this diff to address the nitpick:
-plt.xlabel(f"Max bet (xDai)") +plt.xlabel("Max bet (xDai)")Tools
Ruff
104-104: f-string without any placeholders
Remove extraneous
f
prefix(F541)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- prediction_market_agent_tooling/deploy/betting_strategy.py (2 hunks)
- prediction_market_agent_tooling/markets/omen/omen.py (1 hunks)
- prediction_market_agent_tooling/tools/betting_strategies/kelly_criterion.py (2 hunks)
- prediction_market_agent_tooling/tools/betting_strategies/market_moving.py (2 hunks)
- prediction_market_agent_tooling/tools/betting_strategies/utils.py (1 hunks)
- scripts/mm_vs_kelly_plot.py (1 hunks)
- scripts/sequence_of_kelly_bets_convergence.py (1 hunks)
- tests/markets/omen/test_omen.py (2 hunks)
- tests/markets/test_betting_strategies.py (5 hunks)
Files skipped from review due to trivial changes (1)
- prediction_market_agent_tooling/tools/betting_strategies/utils.py
Additional context used
Ruff
scripts/sequence_of_kelly_bets_convergence.py
109-109: f-string without any placeholders
Remove extraneous
f
prefix(F541)
scripts/mm_vs_kelly_plot.py
47-47: Avoid equality comparisons to
True
; useif market_moving_bet.direction:
for truth checksReplace with
market_moving_bet.direction
(E712)
61-61: Avoid equality comparisons to
True
; useif kelly_bet.direction:
for truth checksReplace with
kelly_bet.direction
(E712)
104-104: f-string without any placeholders
Remove extraneous
f
prefix(F541)
Additional comments not posted (20)
scripts/sequence_of_kelly_bets_convergence.py (5)
11-13
: Clear and concise docstring.The docstring provides a clear and concise explanation of the script's purpose.
16-17
: LGTM!The
get_p_yes_from_token_pool
function is correctly implemented and follows the expected formula for calculating the probability of the "yes" outcome from the token pool.
20-48
: Well-documented and correctly implemented.The
get_buy_amount
function is correctly implemented and follows the derived formula for calculating the buy amount. The docstring provides a clear and detailed explanation of the formula and the variables used.
51-59
: LGTM!The
get_new_token_pool_after_bet
function is correctly implemented and follows the expected logic for updating the token pool after a bet is placed.
62-114
: Simulation and visualization code looks good.The simulation and visualization code is correctly implemented and follows the expected logic for simulating Kelly bets and plotting their convergence to the estimated probability for different bet ratios.
Tools
Ruff
109-109: f-string without any placeholders
Remove extraneous
f
prefix(F541)
scripts/mm_vs_kelly_plot.py (1)
1-26
: LGTM!The imports and market retrieval code are approved.
prediction_market_agent_tooling/tools/betting_strategies/kelly_criterion.py (2)
Line range hint
9-55
: LGTM!The changes to the
get_kelly_bet_simplified
function are approved:
- The renaming of the function and the change in the return type align with the simplification of the betting strategy representation.
- The function correctly calculates the optimal bet amount using the Kelly Criterion formula.
- The function includes checks for valid probabilities and handles edge cases appropriately.
- The function ensures that the bet size is non-negative and does not exceed the wallet balance.
58-137
: LGTM!The addition of the
get_kelly_bet_full
function is approved:
- The function correctly calculates the optimal bet amount using the full Kelly Criterion formula, accounting for the impact of the bet on the market odds.
- The function includes checks for valid probabilities and handles edge cases appropriately.
- The function returns a
SimpleBet
object, which aligns with the simplified representation of the betting strategy.- The function includes the appropriate license information from the original source.
prediction_market_agent_tooling/tools/betting_strategies/market_moving.py (2)
Line range hint
12-85
: LGTM!The changes to the
get_market_moving_bet
function are approved. The updates to the function signature and return type improve the flexibility and clarity of the function. The internal logic has been correctly updated to reflect these changes, and the binary search for the optimal bet amount is implemented correctly.
88-132
: LGTM!The addition of the
_sanity_check_omen_market_moving_bet
function is approved. The function is a useful utility for validating that a bet moves the market to the desired probability. The calculations for the new outcome pool sizes and the new market constant after the bet are correct, and the function correctly checks if the new market probability is close to the target probability.prediction_market_agent_tooling/deploy/betting_strategy.py (1)
173-192
: LGTM!The changes introduce a more nuanced approach to calculating Kelly bets by considering the availability of the token pool. This allows for a more accurate calculation when the token pool is available while still functioning with a simplified calculation when it's not.
The use of
check_not_none
also adds robustness by validating the outcome token pool before use.Overall, the changes improve the betting strategy while maintaining correct functionality.
tests/markets/omen/test_omen.py (1)
215-245
: LGTM!The new test function
test_get_new_p_yes
is well-structured and covers the key aspects of theget_new_p_yes
method:
- It retrieves an open market for testing.
- It asserts that the new probability increases when betting in the "yes" direction and decreases when betting in the "no" direction.
- It performs a sanity check by comparing the calculated new probability against a target probability using a market-moving bet.
The test function relies on the
get_market_moving_bet
function, which is imported from theprediction_market_agent_tooling.tools.betting_strategies.market_moving
module.The code changes are approved.
tests/markets/test_betting_strategies.py (6)
164-193
: LGTM!The changes to the
test_get_market_moving_bet
function improve the clarity and accuracy of the test:
- The parameter names have been updated for better clarity.
- The
get_market_moving_bet
function call now includes additional parameters derived from the market's outcome token amounts, enhancing the accuracy of the bet calculations.- The assertions have been updated to validate the bet direction instead of specific amounts, which is more robust.
195-214
: Great addition!The new
test_sanity_check_market_moving_bet
function enhances the robustness of the betting strategies tests by:
- Using real market data to generate the bets.
- Validating the integrity of the generated bets against the expected probabilities.
- Utilizing the newly added
_sanity_check_omen_market_moving_bet
function.This test ensures that the market moving bets generated are consistent with the expected probabilities.
243-266
: LGTM!The changes to the
test_kelly_bet
function allow for a more nuanced approach to calculating Kelly criterion bets:
- The
get_kelly_bet
function has been replaced with two distinct functions:get_kelly_bet_simplified
andget_kelly_bet_full
, providing both simplified and detailed calculations.- The assertions have been updated to validate the bet direction instead of specific amounts, ensuring the bets are consistent with the expected probabilities.
268-304
: Great addition!The new
test_zero_bets
function ensures that the betting strategies handle edge cases correctly:
- It validates that the
get_market_moving_bet
function returns a bet size close to zero when the target probability is the same as the market probability.- It validates that the
get_kelly_bet_full
function returns a bet size of zero when the max bet is set to zero.- It validates that the
get_kelly_bet_simplified
function returns a bet size of zero when the estimated probability is the same as the market probability.This test enhances the robustness of the betting strategies by ensuring they behave correctly in edge cases where the bets are expected to be zero.
Line range hint
91-123
:
Line range hint
228-239
:prediction_market_agent_tooling/markets/omen/omen.py (2)
580-592
: LGTM!The
get_buy_token_amount
method is implemented correctly. It converts the bet amount to Wei, calls the contract'scalcBuyAmount
function to determine the token amount, and then converts the result back to xDai before returning it.
594-621
: LGTM!The
get_new_p_yes
method is implemented correctly. It calculates the updated probability of a "yes" outcome after a bet is placed by adjusting the outcome pool sizes based on the bet amount and fees, and then computing the new probability from the updated pool sizes. It also properly handles the case when there is no token pool available by raising aValueError
.
) | ||
|
||
|
||
plt.xlabel(f"Bet number") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove extraneous f
prefix.
The Ruff static analysis tool correctly points out that the f-string at line 109 has an extraneous f
prefix without any placeholders.
Remove the f
prefix to fix the issue:
-plt.xlabel(f"Bet number")
+plt.xlabel("Bet number")
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
plt.xlabel(f"Bet number") | |
plt.xlabel("Bet number") |
Tools
Ruff
109-109: f-string without any placeholders
Remove extraneous
f
prefix(F541)
No description provided.