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

137 arbitrage agent #511

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

71 changes: 71 additions & 0 deletions prediction_market_agent/agents/arbitrage_agent/data_models.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import typing as t

from prediction_market_agent_tooling.markets.agent_market import AgentMarket
from pydantic import BaseModel, computed_field


class Correlation(BaseModel):
near_perfect_correlation: bool
reasoning: str


class CorrelatedMarketPair(BaseModel):
main_market: AgentMarket
related_market: AgentMarket
correlation: float

@computed_field # type: ignore[prop-decorator]
@property
def potential_profit_per_bet_unit(self) -> float:
"""
Calculate potential profit per bet unit based on high positive market correlation.
For positively correlated markets: Bet YES/NO or NO/YES.
"""
# Smaller correlations will be handled in a future ticket
# https://github.com/gnosis/prediction-market-agent/issues/508
# Negative correlations are not yet supported by the current LLM prompt, hence not handling those for now.
gabrielfior marked this conversation as resolved.
Show resolved Hide resolved
p_yes = min(self.main_market.current_p_yes, self.related_market.current_p_yes)
p_no = min(self.main_market.current_p_no, self.related_market.current_p_no)
total_probability = p_yes + p_no

# Ensure total_probability is non-zero to avoid division errors
if total_probability > 0:
return (1.0 / total_probability) - 1.0
else:
return 0 # No arbitrage possible if the sum of probabilities is zero

@computed_field # type: ignore[prop-decorator]
@property
def market_to_bet_yes(self) -> AgentMarket:
return (
self.main_market
if self.main_market.current_p_yes <= self.related_market.current_p_yes
else self.related_market
)

@computed_field # type: ignore[prop-decorator]
@property
def market_to_bet_no(self) -> AgentMarket:
return (
self.main_market
if self.main_market.current_p_yes > self.related_market.current_p_yes
else self.related_market
)
gabrielfior marked this conversation as resolved.
Show resolved Hide resolved
Comment on lines +39 to +55
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Correct the logic in market_to_bet_no method

The market_to_bet_yes method looks good. However, the market_to_bet_no method's logic doesn't align with its purpose.

Update the condition in the market_to_bet_no method to compare current_p_no:

     def market_to_bet_no(self) -> AgentMarket:
         return (
             self.main_market
-            if self.main_market.current_p_yes > self.related_market.current_p_yes
+            if self.main_market.current_p_no <= self.related_market.current_p_no
             else self.related_market
         )

This change ensures we're selecting the market with the higher "NO" probability, which is consistent with the method's name and purpose.

📝 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
@computed_field # type: ignore[prop-decorator]
@property
def market_to_bet_yes(self) -> AgentMarket:
return (
self.main_market
if self.main_market.current_p_yes <= self.related_market.current_p_yes
else self.related_market
)
@computed_field # type: ignore[prop-decorator]
@property
def market_to_bet_no(self) -> AgentMarket:
return (
self.main_market
if self.main_market.current_p_yes > self.related_market.current_p_yes
else self.related_market
)
@computed_field # type: ignore[prop-decorator]
@property
def market_to_bet_yes(self) -> AgentMarket:
return (
self.main_market
if self.main_market.current_p_yes <= self.related_market.current_p_yes
else self.related_market
)
@computed_field # type: ignore[prop-decorator]
@property
def market_to_bet_no(self) -> AgentMarket:
return (
self.main_market
if self.main_market.current_p_no <= self.related_market.current_p_no
else self.related_market
)

Comment on lines +48 to +55
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider refactoring for clarity, but current implementation is correct

The current implementation of market_to_bet_no is functionally correct, as it selects the market with the higher "YES" probability for a "NO" bet. However, for consistency and clarity, consider refactoring to directly compare "NO" probabilities.

While the current implementation is correct, you might want to consider the following refactor for improved readability:

     def market_to_bet_no(self) -> AgentMarket:
         return (
             self.main_market
-            if self.main_market.current_p_yes > self.related_market.current_p_yes
+            if self.main_market.current_p_no >= self.related_market.current_p_no
             else self.related_market
         )

This change makes the logic more intuitive by directly comparing "NO" probabilities, which aligns with the method's name and purpose.

📝 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
@computed_field # type: ignore[prop-decorator]
@property
def market_to_bet_no(self) -> AgentMarket:
return (
self.main_market
if self.main_market.current_p_yes > self.related_market.current_p_yes
else self.related_market
)
@computed_field # type: ignore[prop-decorator]
@property
def market_to_bet_no(self) -> AgentMarket:
return (
self.main_market
if self.main_market.current_p_no >= self.related_market.current_p_no
else self.related_market
)


def split_bet_amount_between_yes_and_no(
self, total_bet_amount: float
) -> t.Tuple[float, float]:
"""Splits total bet amount following equations below:
A1/p1 = A2/p2 (same profit regardless of outcome resolution)
A1 + A2 = total bet amount
"""
amount_to_bet_yes = (
total_bet_amount
* self.market_to_bet_yes.current_p_yes
/ (
self.market_to_bet_yes.current_p_yes
+ self.market_to_bet_no.current_p_no
)
)
gabrielfior marked this conversation as resolved.
Show resolved Hide resolved
amount_to_bet_no = total_bet_amount - amount_to_bet_yes
return amount_to_bet_yes, amount_to_bet_no
Comment on lines +57 to +73
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add check for division by zero

The split_bet_amount_between_yes_and_no method correctly implements the formula for splitting the bet amount. However, it's important to handle the case where the denominator could be zero to prevent potential runtime errors.

Add a check to ensure the denominator is not zero before performing the division:

+        denominator = (
+            self.market_to_bet_yes.current_p_yes
+            + self.market_to_bet_no.current_p_no
+        )
+        if denominator == 0:
+            raise ValueError("Sum of probabilities is zero, cannot split bet amount")
         amount_to_bet_yes = (
             total_bet_amount
             * self.market_to_bet_yes.current_p_yes
-            / (
-                self.market_to_bet_yes.current_p_yes
-                + self.market_to_bet_no.current_p_no
-            )
+            / denominator
         )

This change will prevent division by zero errors and provide a clear error message when the bet amount cannot be split due to zero probabilities.

The rest of the implementation looks good and correctly calculates the split bet amounts.

📝 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
def split_bet_amount_between_yes_and_no(
self, total_bet_amount: float
) -> t.Tuple[float, float]:
"""Splits total bet amount following equations below:
A1/p1 = A2/p2 (same profit regardless of outcome resolution)
A1 + A2 = total bet amount
"""
amount_to_bet_yes = (
total_bet_amount
* self.market_to_bet_yes.current_p_yes
/ (
self.market_to_bet_yes.current_p_yes
+ self.market_to_bet_no.current_p_no
)
)
amount_to_bet_no = total_bet_amount - amount_to_bet_yes
return amount_to_bet_yes, amount_to_bet_no
def split_bet_amount_between_yes_and_no(
self, total_bet_amount: float
) -> t.Tuple[float, float]:
"""Splits total bet amount following equations below:
A1/p1 = A2/p2 (same profit regardless of outcome resolution)
A1 + A2 = total bet amount
"""
denominator = (
self.market_to_bet_yes.current_p_yes
+ self.market_to_bet_no.current_p_no
)
if denominator == 0:
raise ValueError("Sum of probabilities is zero, cannot split bet amount")
amount_to_bet_yes = (
total_bet_amount
* self.market_to_bet_yes.current_p_yes
/ denominator
)
amount_to_bet_no = total_bet_amount - amount_to_bet_yes
return amount_to_bet_yes, amount_to_bet_no

179 changes: 179 additions & 0 deletions prediction_market_agent/agents/arbitrage_agent/deploy.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
import typing as t

from langchain_core.output_parsers import PydanticOutputParser
from langchain_core.prompts import PromptTemplate
from langchain_core.runnables import RunnableSerializable
from langchain_openai import ChatOpenAI
from prediction_market_agent_tooling.deploy.agent import (
MAX_AVAILABLE_MARKETS,
DeployableTraderAgent,
)
from prediction_market_agent_tooling.gtypes import Probability
from prediction_market_agent_tooling.loggers import logger
from prediction_market_agent_tooling.markets.agent_market import (
AgentMarket,
FilterBy,
SortBy,
)
from prediction_market_agent_tooling.markets.data_models import (
BetAmount,
Position,
ProbabilisticAnswer,
TokenAmount,
Trade,
TradeType,
)
from prediction_market_agent_tooling.markets.markets import MarketType
from prediction_market_agent_tooling.markets.omen.omen import OmenAgentMarket
from prediction_market_agent_tooling.markets.omen.omen_subgraph_handler import (
OmenSubgraphHandler,
)
from prediction_market_agent_tooling.tools.langfuse_ import (
get_langfuse_langchain_config,
observe,
)

from prediction_market_agent.agents.arbitrage_agent.data_models import (
CorrelatedMarketPair,
Correlation,
)
from prediction_market_agent.agents.arbitrage_agent.prompt import prompt_template
from prediction_market_agent.db.pinecone_handler import PineconeHandler
from prediction_market_agent.utils import APIKeys


class DeployableOmenArbitrageAgent(DeployableTraderAgent):
"""Agent that places mirror bets on Omen for (quasi) risk-neutral profit."""

model = "gpt-4o-mini"
gabrielfior marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider making the model attribute configurable

The model attribute is currently hardcoded to "gpt-4o-mini". To improve flexibility and allow for easier updates or testing with different models, consider making it a configurable parameter, either through the constructor or a configuration file.


def load(self) -> None:
self.subgraph_handler = OmenSubgraphHandler()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason you've made this agent omen-specific? Looks like the only thing you've used the sgh for is to get markets. So it looks like this could easily be changed to become market-platform agnostic

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made it Omen-specific because the related_markets should be queryable via Pinecone (because of the TTA agent), and only Omen agents are stored there currently.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I didn't realise that. Where does the indexing of omen markets in the pincone db happen? If it's not part of this agent, it goes against @kongzii's philosophy of "anyone should be able to run the agents (if they have appropriate api keys), and get the same kind of results".

Regardless, there doesn't seem to be a need to have any Omen dependencies here - you could just have a if market_type != MarketType.Omen: raise ... check

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indexing occurs below

self.pinecone_handler.insert_all_omen_markets_if_not_exists(

I don't think it prevents someone from running the agent - you just need Pinecone api keys and, on the first run, it will fill out Pinecone and have the same dataset as we do.

Regardless, there doesn't seem to be a need to have any Omen dependencies here - you could just have a if market_type != MarketType.Omen: raise ... check

Makes sense to me, implementing this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well it does if the user wants to run just this agent, and not the DeployableThinkThoroughlyAgent as well! I think we need to do self.pinecone_handler.insert_all_omen_markets_if_not_exists(...) somewhere here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure - added 55aee24

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just had a look at what pinecone_handler.update_markets is doing. Is this what we want? Two things I'm not sure about:

  1. It doesn't look like it's removing closed markets from the index
  2. It only fetches the last 7 days' worth of created markets created_after = utcnow() - datetime.timedelta(days=7). Don't we want all open markets?

self.pinecone_handler = PineconeHandler()
self.chain = self._build_chain()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding error handling to the load method

The load method initializes important components of the agent. To improve robustness, consider adding error handling to catch and log any exceptions that might occur during initialization of subgraph_handler, pinecone_handler, or chain.


def get_markets(
self,
market_type: MarketType,
limit: int = MAX_AVAILABLE_MARKETS,
sort_by: SortBy = SortBy.CLOSING_SOONEST,
filter_by: FilterBy = FilterBy.OPEN,
) -> t.Sequence[AgentMarket]:
return super().get_markets(
market_type=market_type,
limit=100,
sort_by=SortBy.HIGHEST_LIQUIDITY,
# Fetching most liquid markets since more likely they will have related markets
filter_by=FilterBy.OPEN,
)
gabrielfior marked this conversation as resolved.
Show resolved Hide resolved

def answer_binary_market(self, market: AgentMarket) -> ProbabilisticAnswer | None:
return ProbabilisticAnswer(p_yes=Probability(0.5), confidence=1.0)

Comment on lines +79 to +81
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Implement actual market analysis or add a TODO comment.

The answer_binary_market method currently returns a fixed 50% probability with full confidence for all markets. This doesn't provide any real market analysis. If this is a placeholder, consider adding a TODO comment explaining the intended future implementation. If not, implement actual market analysis logic.

Example:

def answer_binary_market(self, market: AgentMarket) -> ProbabilisticAnswer | None:
    # TODO: Implement actual market analysis logic
    # For now, returning a default 50% probability with full confidence
    return ProbabilisticAnswer(p_yes=Probability(0.5), confidence=1.0)

def _build_chain(self) -> RunnableSerializable[t.Any, t.Any]:
llm = ChatOpenAI(
temperature=0,
model=self.model,
api_key=APIKeys().openai_api_key_secretstr_v1,
)

parser = PydanticOutputParser(pydantic_object=Correlation)
prompt = PromptTemplate(
template=prompt_template,
input_variables=["main_market_question", "related_market_question"],
partial_variables={"format_instructions": parser.get_format_instructions()},
)

chain = prompt | llm | parser
return chain
Comment on lines +82 to +97
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider making the temperature configurable.

The _build_chain method uses a hardcoded temperature of 0 for the ChatOpenAI model. To improve flexibility and allow for different levels of randomness in the model's output, consider making the temperature a configurable parameter. You could add it as a class attribute or a parameter to the method.

Example:

def _build_chain(self, temperature: float = 0) -> RunnableSerializable[t.Any, t.Any]:
    llm = ChatOpenAI(
        temperature=temperature,
        model=self.model,
        api_key=APIKeys().openai_api_key_secretstr_v1,
    )
    # ... rest of the method

This change would allow for easier experimentation with different temperature settings without modifying the code.


@observe()
def calculate_correlation_between_markets(
gabrielfior marked this conversation as resolved.
Show resolved Hide resolved
self, market: AgentMarket, related_market: AgentMarket
) -> Correlation:
correlation: Correlation = self.chain.invoke(
{
"main_market_question": market.question,
"related_market_question": related_market.question,
}
)
return correlation
Comment on lines +99 to +109
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling for chain invocation.

The calculate_correlation_between_markets method invokes the language model chain without any error handling. Consider adding a try-except block to catch and handle potential exceptions from the API call. This will improve the robustness of the method and prevent potential crashes due to network issues or API failures.

Example implementation:

@observe()
def calculate_correlation_between_markets(
    self, market: AgentMarket, related_market: AgentMarket
) -> Correlation:
    try:
        correlation: Correlation = self.chain.invoke(
            {
                "main_market_question": market.question,
                "related_market_question": related_market.question,
            }
        )
        return correlation
    except Exception as e:
        logger.error(f"Error calculating correlation: {str(e)}")
        # You might want to return a default correlation or re-raise the exception
        raise

This change would make the method more robust to potential failures in the chain invocation.


@observe()
def get_correlated_markets(self, market: AgentMarket) -> list[CorrelatedMarketPair]:
# We try to find similar, open markets which point to the same outcome.
correlated_markets = []
related = self.pinecone_handler.find_nearest_questions_with_threshold(
limit=10, text=market.question
)

omen_markets = self.subgraph_handler.get_omen_binary_markets(
limit=len(related),
id_in=[i.market_address.lower() for i in related],
resolved=False,
)

# Note that negative correlation is hard - e.g. for the US presidential election, markets on each candidate are not seen as -100% correlated.
for related_market in omen_markets:
result: Correlation = self.chain.invoke(
{
"main_market_question": market,
"related_market_question": related_market,
Comment on lines +129 to +130
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Incorrect arguments passed to self.chain.invoke

In the get_correlated_markets method, you're passing market and related_market objects to self.chain.invoke, whereas the chain expects strings containing the market questions.

Apply this diff to fix the arguments:

-                "main_market_question": market,
-                "related_market_question": related_market,
+                "main_market_question": market.question,
+                "related_market_question": related_market.question,
📝 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
"main_market_question": market,
"related_market_question": related_market,
"main_market_question": market.question,
"related_market_question": related_market.question,

},
config=get_langfuse_langchain_config(),
)
if related_market.id != market.id and result.near_perfect_correlation:
related_agent_market = OmenAgentMarket.from_data_model(related_market)
correlated_markets.append(
CorrelatedMarketPair(
main_market=market,
related_market=related_agent_market,
correlation=result.correlation,
)
)
return correlated_markets
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Fix arguments passed to self.chain.invoke

In the get_correlated_markets method, you're passing market and related_market objects to self.chain.invoke, whereas the chain expects strings containing the market questions. Update the code as follows:

 result: Correlation = self.chain.invoke(
     {
-        "main_market_question": market,
-        "related_market_question": related_market,
+        "main_market_question": market.question,
+        "related_market_question": related_market.question,
     },
     config=get_langfuse_langchain_config(),
 )

This change ensures that the correct data is passed to the language model for correlation analysis.

Add error handling for external API calls

The get_correlated_markets method makes several external API calls (to Pinecone, Omen subgraph, and the language model) without any error handling. Consider adding try-except blocks to catch and handle potential exceptions from these calls. This will improve the robustness of the method and prevent potential crashes due to network issues or API failures.

📝 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
@observe()
def get_correlated_markets(self, market: AgentMarket) -> list[CorrelatedMarketPair]:
# We try to find similar, open markets which point to the same outcome.
correlated_markets = []
related = self.pinecone_handler.find_nearest_questions_with_threshold(
limit=10, text=market.question
)
omen_markets = self.subgraph_handler.get_omen_binary_markets(
limit=len(related),
id_in=[i.market_address.lower() for i in related],
resolved=False,
)
# Note that negative correlation is hard - e.g. for the US presidential election, markets on each candidate are not seen as -100% correlated.
for related_market in omen_markets:
result: Correlation = self.chain.invoke(
{
"main_market_question": market,
"related_market_question": related_market,
},
config=get_langfuse_langchain_config(),
)
if related_market.id != market.id and result.near_perfect_correlation:
related_agent_market = OmenAgentMarket.from_data_model(related_market)
correlated_markets.append(
CorrelatedMarketPair(
main_market=market,
related_market=related_agent_market,
correlation=result.correlation,
)
)
return correlated_markets
@observe()
def get_correlated_markets(self, market: AgentMarket) -> list[CorrelatedMarketPair]:
# We try to find similar, open markets which point to the same outcome.
correlated_markets = []
related = self.pinecone_handler.find_nearest_questions_with_threshold(
limit=10, text=market.question
)
omen_markets = self.subgraph_handler.get_omen_binary_markets(
limit=len(related),
id_in=[i.market_address.lower() for i in related],
resolved=False,
)
# Note that negative correlation is hard - e.g. for the US presidential election, markets on each candidate are not seen as -100% correlated.
for related_market in omen_markets:
result: Correlation = self.chain.invoke(
{
"main_market_question": market.question,
"related_market_question": related_market.question,
},
config=get_langfuse_langchain_config(),
)
if related_market.id != market.id and result.near_perfect_correlation:
related_agent_market = OmenAgentMarket.from_data_model(related_market)
correlated_markets.append(
CorrelatedMarketPair(
main_market=market,
related_market=related_agent_market,
correlation=result.correlation,
)
)
return correlated_markets


@observe()
def build_trades_for_correlated_markets(
self, pair: CorrelatedMarketPair
) -> list[Trade]:
market_to_bet_yes, market_to_bet_no = pair.main_market, pair.related_market
total_amount: BetAmount = pair.main_market.get_tiny_bet_amount()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like something that should be configurable at the top level

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented

# Split between main_market and related_market
amount_yes, amount_no = pair.split_bet_amount_between_yes_and_no(
total_amount.amount
)
trades = [
Trade(
trade_type=TradeType.BUY,
outcome=True,
amount=TokenAmount(
amount=amount_yes, currency=market_to_bet_yes.currency
),
),
Trade(
trade_type=TradeType.BUY,
outcome=False,
amount=TokenAmount(
amount=amount_no, currency=market_to_bet_no.currency
),
),
Comment on lines +155 to +168
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Buying [market1: yes, market2: no] assumes positive correlation between markets. I think you need to have some logic that chooses the directions based on correlation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Positive correlation is assumed here -> https://github.com/gnosis/prediction-market-agent/pull/511/files/433656d955694050935c662e1574f80c552d3cc7#diff-1b9d8df9eb7cb55918e642ebc0f26902d5211d4946a8c4d0454e9889dd05c371R138-R140

I didn't implement logic for the negatively correlated case (it was on an earlier version), I think positive correlation is a good start. Also, negative correlation is harder to determine, e.g. Trump president vs Kamala president is 100% negatively correlated but not trivial for LLM to tell, hence preferred to stick to 100% correlation case.

Happy with suggestions though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh okay. Fwiw I think it's okay if the LLMs find it hard to determine that, as long as there aren't false positives.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going with your bool suggestion from the other thread.

]
logger.info(f"Placing arbitrage trades {trades}")
return trades
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider making the bet amount configurable.

The build_trades_for_correlated_markets method currently uses a fixed "tiny bet amount" for trades. To improve flexibility and allow for different trading strategies, consider making this bet amount configurable. You could add a parameter to the method or use a class attribute that can be set during initialization or runtime.


@observe()
def build_trades(
self,
market: AgentMarket,
answer: ProbabilisticAnswer,
existing_position: Position | None,
) -> list[Trade]:
trades = []
correlated_markets = self.get_correlated_markets(market=market)
for pair in correlated_markets:
if pair.potential_profit_per_bet_unit > 0:
trades_for_pair = self.build_trades_for_correlated_markets(pair)
trades.extend(trades_for_pair)

return trades
Comment on lines +173 to +187
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider utilizing or removing unused parameters in build_trades.

The build_trades method currently ignores the answer and existing_position parameters. If these parameters are not needed for the arbitrage strategy, consider removing them to avoid confusion. If they could be useful, consider incorporating them into your trade building logic.

If the parameters are not needed:

@observe()
def build_trades(
    self,
    market: AgentMarket,
) -> list[Trade]:
    trades = []
    correlated_markets = self.get_correlated_markets(market=market)
    for pair in correlated_markets:
        if pair.potential_profit_per_bet_unit > 0:
            trades_for_pair = self.build_trades_for_correlated_markets(pair)
            trades.extend(trades_for_pair)
    return trades

If you want to utilize these parameters, consider adjusting the trade amount based on the answer's confidence or the existing position.

The overall logic of the method for building trades based on correlated markets with potential profit is correct.

12 changes: 12 additions & 0 deletions prediction_market_agent/agents/arbitrage_agent/prompt.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
prompt_template = """Given two markets, MARKET 1 and MARKET 2, provide a boolean value that represents the correlation between these two markets' outcomes. Return True if the outcomes are perfectly or nearly perfectly correlated, meaning there is a high probability that both markets resolve to the same outcome. Return False if the correlation is weak or non-existent.
Correlation can also be understood as the conditional probability that market 2 resolves to YES, given that market 1 resolved to YES.
In addition to the boolean value, explain the reasoning behind your decision.
[MARKET 1]
{main_market_question}
[MARKET 2]
{related_market_question}
Follow the formatting instructions below for producing an output in the correct format.
{format_instructions}"""
Comment on lines +1 to +12
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Refine the prompt template for more accurate correlation representation.

While the current template provides a framework for assessing market correlations, there are several areas where it could be improved:

  1. Boolean output vs. float: The current template asks for a boolean value, which oversimplifies the concept of correlation. Consider reverting to a float between -1 and 1 for more nuanced representation.

  2. Correlation definition: The definition focusing on conditional probability may be misleading. Consider including a more comprehensive explanation of correlation.

  3. Handling of negative correlations and unrelated markets: The template doesn't provide guidance on these scenarios. Include explanations for these cases.

Consider updating the prompt template to address these points. Here's a suggested improvement:

prompt_template = """Given two prediction markets, MARKET 1 and MARKET 2, provide a float value between -1 and 1 that represents the correlation between these two markets' outcomes.

Correlation measures the strength and direction of the relationship between the outcomes of the two markets:
- A correlation of 1 means the markets are perfectly positively correlated (if market 1 is YES, market 2 is very likely to be YES).
- A correlation of -1 means the markets are perfectly negatively correlated (if market 1 is YES, market 2 is very likely to be NO).
- A correlation of 0 means the markets are not correlated (the outcome of market 1 does not influence market 2).

If the markets appear to be completely unrelated, output 0.

[MARKET 1]
{main_market_question}

[MARKET 2]
{related_market_question}

Analyze the relationship between these markets and provide your correlation estimate. In addition to the float value, explain the reasoning behind your decision.

Follow the formatting instructions below for producing an output in the correct format.
{format_instructions}"""

This updated template provides a more comprehensive explanation of correlation and how to interpret different values, which should lead to more accurate and consistent results.

Empty file.
63 changes: 63 additions & 0 deletions tests/agents/arbitrage_agent/test_arbitrage_agent.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import typing as t
from unittest.mock import Mock, patch

import pytest
from prediction_market_agent_tooling.markets.agent_market import AgentMarket
from prediction_market_agent_tooling.markets.omen.omen import OmenAgentMarket

from prediction_market_agent.agents.arbitrage_agent.deploy import (
DeployableOmenArbitrageAgent,
)


@pytest.fixture(scope="module")
def arbitrage_agent() -> t.Generator[DeployableOmenArbitrageAgent, None, None]:
with patch(
"prediction_market_agent.agents.arbitrage_agent.deploy.DeployableOmenArbitrageAgent.load",
new=lambda x: None,
), patch(
"prediction_market_agent_tooling.tools.langfuse_.get_langfuse_langchain_config"
):
agent = DeployableOmenArbitrageAgent()
# needed since load was mocked
agent.chain = agent._build_chain()
yield agent


@pytest.fixture(scope="module")
def main_market() -> t.Generator[AgentMarket, None, None]:
m1 = Mock(OmenAgentMarket, wraps=OmenAgentMarket)
m1.question = "Will Kamala Harris win the US presidential election in 2024?"
yield m1


@pytest.fixture(scope="module")
def related_market() -> t.Generator[AgentMarket, None, None]:
m1 = Mock(OmenAgentMarket, wraps=OmenAgentMarket)
m1.question = "Will Kamala Harris become the US president in 2025?"
Copy link
Contributor

@evangriffiths evangriffiths Oct 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused by this, if you expect high correlation with the other question shouldn't this be either:

  • "Will Kamala Harris be the US president in 2025?" or
  • "Will Kamala Harris become the US president in 2024?"
    because the election is in Nov 2024

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh scrap that sorry, elected president takes office in Jan 2025. Forgot!

yield m1


@pytest.fixture(scope="module")
def unrelated_market() -> t.Generator[AgentMarket, None, None]:
m1 = Mock(OmenAgentMarket, wraps=OmenAgentMarket)
m1.question = "Will Donald Duck ever retire from his adventures in Duckburg?"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😆

yield m1


@pytest.mark.parametrize(
"related_market_fixture_name, is_correlated",
[("related_market", True), ("unrelated_market", False)],
)
def test_correlation_for_similar_markets(
arbitrage_agent: DeployableOmenArbitrageAgent,
main_market: AgentMarket,
related_market_fixture_name: str,
is_correlated: bool,
request: pytest.FixtureRequest,
) -> None:
other_market = request.getfixturevalue(related_market_fixture_name)
correlation = arbitrage_agent.calculate_correlation_between_markets(
market=main_market, related_market=other_market
)
assert correlation.near_perfect_correlation == is_correlated
Loading