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

Support for 2+ outcomes #499

Closed
wants to merge 2 commits into from
Closed

Support for 2+ outcomes #499

wants to merge 2 commits into from

Conversation

Chanka0
Copy link

@Chanka0 Chanka0 commented May 4, 2022

Description

Implements basic support for predictions with more than two potential outcomes. Part of the fix included modifying how choices were interpreted by changing decision["choice"] to be the index rather than a letter being used as intermediary.

Basic safeguards for the divide by zero error were added in as well.

Fixes #497
Limited support for predictions with more than 2 outcomes.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

This has been tested 4-5 times on predictions with more than 2 outcomes, most of which utilizing 10 (the new limit).

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented on my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (README.md)
  • My changes generate no new warnings
  • Any dependent changes have been updated in requirements.txt

@Chanka0
Copy link
Author

Chanka0 commented May 4, 2022

One note to be added: The SMART strategy has not been converted to support more than 2 strategies.

@Tkd-Alex Tkd-Alex added the enhancement New feature or request label May 4, 2022
@Rakambda
Copy link
Contributor

Rakambda commented May 9, 2022

IMO the smart strategy should probably take the 2 outcomes with the most people rather than the 2 first (which is obviously random).

Based on most people is a decision to make but I think that it makes sense. Smart used to take the choice with most people but if these are too close (~ almost same chances) it takes the one with most potential gain.
With multiple choices it'd do basically the same. The 2 choices with most people are the 2 that stands out the most and seem the most likely. If they're close, then pick with best gain.

That's what I did in my fork, if I have some time to spare I'll try to gather stats on how it behaves.

@lazinism
Copy link

lazinism commented May 27, 2022

Currently I changed line 158~ in .\classes\entities\Bet.py like this:
self.total_users = ( sum(map(lambda x:x[OutcomeKeys.TOTAL_USERS],self.outcomes)) #self.outcomes[0][OutcomeKeys.TOTAL_USERS] #+ self.outcomes[1][OutcomeKeys.TOTAL_USERS] )
self.total_points = ( sum(map(lambda x:x[OutcomeKeys.TOTAL_POINTS],self.outcomes)) #reduce(lambda x,y:x[OutcomeKeys.TOTAL_POINTS]+y[OutcomeKeys.TOTAL_POINTS],self.outcomes) #self.outcomes[0][OutcomeKeys.TOTAL_POINTS] #+ self.outcomes[1][OutcomeKeys.TOTAL_POINTS] )
if ( self.total_users > 0 and all(map(lambda x:x[OutcomeKeys.TOTAL_POINTS]>0, self.outcomes)) #and self.outcomes[0][OutcomeKeys.TOTAL_POINTS] > 0 #and self.outcomes[1][OutcomeKeys.TOTAL_POINTS] > 0 ):
And I think it seems to work
maybe there's some other codes that only counts 2 selections but I can't find it now

@rdavydov
Copy link

@lazinism by removing comments and formatting your code, we get:

self.total_users = ( sum(map(lambda x:x[OutcomeKeys.TOTAL_USERS],self.outcomes))
self.total_points = ( sum(map(lambda x:x[OutcomeKeys.TOTAL_POINTS],self.outcomes))
if ( self.total_users > 0 and all(map(lambda x:x[OutcomeKeys.TOTAL_POINTS]>0, self.outcomes)) ):

Right?

@rdavydov
Copy link

@lazinism Do we really need to use all(map()) in the condition? Can't we just use:

if ( self.total_users > 0 and self.total_points > 0 ):

?

@rdavydov
Copy link

@lazinism Your solution with sum(), map() and lambda-functions is just an elegant way of doing:

        #inefficient but otherwise outcomekeys are represented wrong.
        self.total_points = 0
        self.total_users = 0
        for index in range(0, len(self.outcomes)):
            self.total_users += self.outcomes[index][OutcomeKeys.TOTAL_USERS]
            self.total_points += self.outcomes[index][OutcomeKeys.TOTAL_POINTS]

right? It is basically the same thing, or am I missing something?

@lazinism
Copy link

Yes I think it's same
I just prefer shorter code, so that i used sum, map, lambda.

@rdavydov
Copy link

@lazinism Do we really need to use:

if ( self.total_users > 0 and all(map(lambda x:x[OutcomeKeys.TOTAL_POINTS]>0, self.outcomes)) ):

Can't we just use:

if ( self.total_users > 0 and self.total_points > 0 ):

?

@lazinism
Copy link

lazinism commented Nov 30, 2022

I wrote it like that because Original code was

self.total_users > 0 and self.outcomes[0][OutcomeKeys.TOTAL_POINTS] > 0 and self.outcomes[1][OutcomeKeys.TOTAL_POINTS] > 0

and I can't predict what would happen if I change code to self.total_points > 0
If that works fine, I think we can change it, maybe

@lazinism
Copy link

so the purpose of the changed code I wrote was to preserve as many original code as i can, but change it from considering only 2 options to multiple options. In that view, you can understand why I wrote code like that 👍

@rdavydov rdavydov mentioned this pull request Dec 9, 2022
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for more than two options in predictions
5 participants