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

Feat/benchmarking convergence #382

Open
wants to merge 22 commits into
base: develop
Choose a base branch
from

Conversation

cyberosa
Copy link
Collaborator

@cyberosa cyberosa commented Jan 9, 2025

This branch fixes the Benchmarking mode running at the trader. It introduces some changes regarding

  • bets queue mechanism
  • nr mech calls limit support

@Adamantios Adamantios force-pushed the feat/benchmarking-convergence branch from 3e985ad to a7e01fd Compare January 13, 2025 11:38
@Adamantios Adamantios marked this pull request as ready for review January 13, 2025 12:05
Comment on lines +315 to +320
elif "id" in data_attributes:
common_attributes = set(bet_annotations) & set(data_attributes)
data = {key: data[key] for key in common_attributes}
if "queue_status" in data:
data["queue_status"] = QueueStatus(data["queue_status"])
return Bet(**data)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you explain this part?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please also add comments to explain this in the code.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Adamantios it was during the time we had a discussion that bet.json didn't have backward compatibility and all you and Anna were still working on the sell token feature and were going to handle this later. basically the new investments field in Bets dataclass was also being treated as a dataclass instance and files couldn't be read during benchmarking mode.

@annasambrook this can be replaced with the current logic of bets on main, this was a backport fix for then because this branch was created out of sell token feature branch that had investments as a dict with investment values for both yes and no.

I can see that chnage has been reverted so this code is not needed.

annasambrook
annasambrook previously approved these changes Jan 13, 2025
@annasambrook annasambrook self-requested a review January 13, 2025 12:13
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.

4 participants