-
Notifications
You must be signed in to change notification settings - Fork 5
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
Implement PredictProbabilityForQuestion function for microchain agent #75
Implement PredictProbabilityForQuestion function for microchain agent #75
Conversation
WalkthroughThe latest update brings significant enhancements to the Changes
Recent Review DetailsConfiguration used: CodeRabbit UI Files ignored due to path filters (1)
Files selected for processing (2)
Files skipped from review as they are similar to previous changes (2)
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
pyproject.toml
Outdated
pydantic-settings = "^2.1.0" | ||
autoflake = "^2.2.1" | ||
isort = "^5.13.2" | ||
markdownify = "^0.11.6" | ||
tavily-python = "^0.3.1" | ||
# TODO remove when PR is merged | ||
microchain-python = {git = "https://github.com/evangriffiths/microchain.git", rev = "evan/token-tracker"} | ||
types-requests = "2.31.0.0" | ||
crewai = {version = "0.11.0", extras = ["tools"]} |
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.
crewai conflict with mech-client deps
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.
I assume this will also be reverted back to v0.28 once PMAT adds mech-client as dependency?
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.
Latest crewai is also incompatible with mech-client, so either need to downgrade crewai, upgrade the deps of mech-client/its deps, or make our own client!
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.
mech-client is super very strictly versioned. if they do this in other repos as well then it's even worse 🙈 I think
it's worth a try to replace these ==
(exact version) with ^
(any newer version except for the versions with breaking changes).
Maybe there still will be some conflicts but a lot less, at least hopefully.
https://github.com/valory-xyz/mech-client/blob/main/pyproject.toml#L27
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.
Unfortunately doesn't help 😞
The conflicts come from further down the dependency tree
- here: https://github.com/valory-xyz/open-aea/blob/1bb984ba44b028d941266ffeb13558bb4f32645d/plugins/aea-ledger-ethereum/setup.py#L46
- and here https://github.com/valory-xyz/open-aea/blob/1bb984ba44b028d941266ffeb13558bb4f32645d/setup.py#L63
So there's strictness to fix first in https://github.com/valory-xyz/open-aea and then in the mech-client repo
def __call__(self, question: str) -> float: | ||
private_key = APIKeys().bet_from_private_key.get_secret_value() | ||
with saved_str_to_tmpfile(private_key) as tmpfile_path: | ||
response = interact( |
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.
This costs money to run. Should we do some kind of balance check here to see if the agent has enough funds? Otherwise this will throw an error.
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.
Added. I don't think there's a way to query the cost of a mech call, so I've hard-coded it here.
I tried playing around with the value here that sets the cost of the mech call, and it fails for anything <0.01xDai, and works for anything >=.
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
pydantic-settings = "^2.1.0" | ||
autoflake = "^2.2.1" | ||
isort = "^5.13.2" | ||
markdownify = "^0.11.6" | ||
tavily-python = "^0.3.1" | ||
# TODO remove when PR is merged | ||
microchain-python = {git = "https://github.com/evangriffiths/microchain.git", rev = "evan/token-tracker"} | ||
setuptools = "^69.5.1" |
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.
Required by mech-client, although not a dependency. Will make a PR against the aea repo, but for now adding here
from aea.crypto.plugin import load_all_plugins
File "/Users/evan/dev/prediction-market-agent/.venv/lib/python3.10/site-packages/aea/crypto/plugin.py", line 26, in <module>
from pkg_resources import EntryPoint, WorkingSet
ModuleNotFoundError: No module named 'pkg_resources'
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
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
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
@@ -141,26 +140,6 @@ def has_known_result(self) -> bool: | |||
""" | |||
|
|||
|
|||
def completion_str_to_json(completion: str) -> dict[str, t.Any]: |
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.
Moved to prediction_market_agent/utils.py
pyproject.toml
Outdated
pydantic-settings = "^2.1.0" | ||
autoflake = "^2.2.1" | ||
isort = "^5.13.2" | ||
markdownify = "^0.11.6" | ||
tavily-python = "^0.3.1" | ||
# TODO remove when PR is merged | ||
microchain-python = {git = "https://github.com/evangriffiths/microchain.git", rev = "evan/token-tracker"} | ||
setuptools = "^69.5.1" | ||
jsonschema = "4.3.3" |
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.
Same here
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: 2
question = self.market_type.market_class.get_binary_market( | ||
id=market_id | ||
).question | ||
private_key = APIKeys().bet_from_private_key.get_secret_value() |
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.
Storing private keys temporarily in files can be risky. Consider using more secure methods to handle private keys, such as using environment variables or secure vault solutions.
- uses: ./.github/actions/python_prepare | ||
- name: Run pytest | ||
run: poetry run pytest | ||
run: poetry run pytest tests |
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.
Don't pick up the tests from inside the mech submodule
"tavily": tavily_api_key, | ||
}, | ||
} | ||
response = prediction_with_research_report.run(**kwargs) |
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.
Why **kwargs and not just
prediction_with_research_report.run(
tool="prediction-with-research-conservative",
prompt=question,
api_keys={
"openai": openai_api_key,
"tavily": tavily_api_key,
},
)
?
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.
Une bonne recommandation!
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.
So because the mech calling over the internet doesn't work very well, this adds it as a submodule and use directly?
Yes, that's right. I have yet to debug why it's unreliable. |
Two new Functions for microchain agent:
PredictProbabilityForQuestion
that calls the mech-client. However, from experimentation this is not very reliable. Currently it fails with timeout ~50% of the time. So while this is being debugged...PredictProbabilityForQuestionLocal
that calls the mech function's python directly. This pulls in the mech repo as a git submodule.Summary by CodeRabbit
pyproject.toml
and GitHub CI workflows..gitignore
to exclude specific files and added essential libraries to the project setup.