Skip to content
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

Closed
wants to merge 18 commits into from

Conversation

evangriffiths
Copy link
Contributor

No description provided.

Copy link

coderabbitai bot commented Sep 12, 2024

Walkthrough

The 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 Omen class for handling token amounts and probabilities, and the addition of a SimpleBet class for better representation of betting actions. Additionally, new scripts for visualizing and simulating betting strategies are introduced, along with comprehensive test coverage to ensure the robustness of the new functionalities.

Changes

Files Change Summary
prediction_market_agent_tooling/deploy/betting_strategy.py, prediction_market_agent_tooling/tools/betting_strategies/kelly_criterion.py Introduced get_kelly_bet_full and get_kelly_bet_simplified functions for improved Kelly criterion calculations. Removed KellyBet class and renamed get_kelly_bet. Enhanced logic to validate token pools and handle edge cases.
prediction_market_agent_tooling/markets/omen/omen.py Added get_buy_token_amount and get_new_p_yes methods to the Omen class, allowing dynamic updates to token amounts and probabilities based on betting actions.
prediction_market_agent_tooling/tools/betting_strategies/market_moving.py Modified get_market_moving_bet to accept outcome pool sizes and probabilities directly, returning a SimpleBet object. Introduced a helper function for sanity checks on market-moving bets.
prediction_market_agent_tooling/tools/betting_strategies/utils.py Introduced SimpleBet class for structured representation of betting actions, utilizing Pydantic for validation.
scripts/mm_vs_kelly_plot.py, scripts/sequence_of_kelly_bets_convergence.py Implemented scripts for visualizing the comparison of Kelly and market-moving bets and simulating the convergence of Kelly betting strategies, respectively.
tests/markets/omen/test_omen.py, tests/markets/test_betting_strategies.py Added tests for new functionalities, including validation of get_new_p_yes and enhancements to existing betting strategy tests to align with the new approach to calculating bets.

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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a 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; use if market_moving_bet.direction: for truth checks.
  • Line 61: Avoid equality comparisons to True; use if 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.size
Tools
Ruff

47-47: Avoid equality comparisons to True; use if market_moving_bet.direction: for truth checks

Replace with market_moving_bet.direction

(E712)


61-61: Avoid equality comparisons to True; use if kelly_bet.direction: for truth checks

Replace 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

Commits

Files that changed from the base of the PR and between 43b39d1 and c90f58b.

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; use if market_moving_bet.direction: for truth checks

Replace with market_moving_bet.direction

(E712)


61-61: Avoid equality comparisons to True; use if kelly_bet.direction: for truth checks

Replace 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 the get_new_p_yes method:

  1. It retrieves an open market for testing.
  2. It asserts that the new probability increases when betting in the "yes" direction and decreases when betting in the "no" direction.
  3. 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 the prediction_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 and get_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's calcBuyAmount 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 a ValueError.

)


plt.xlabel(f"Bet number")
Copy link

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.

Suggested change
plt.xlabel(f"Bet number")
plt.xlabel("Bet number")
Tools
Ruff

109-109: f-string without any placeholders

Remove extraneous f prefix

(F541)

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.

1 participant