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

Added Epsilion Greedy #219

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Added Epsilion Greedy #219

wants to merge 1 commit into from

Conversation

JoeHolt
Copy link

@JoeHolt JoeHolt commented Oct 16, 2018

Added basic epsilion greedy to CardinalBandits

Copy link
Member

@stsievert stsievert left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @JoeHolt!

@@ -42,22 +42,28 @@ initExp:
default: 0.5
optional: true

pT:
Copy link
Member

Choose a reason for hiding this comment

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

Why is this required? The YAML file is for algorithm inputs/outputs. pT is internal to the algorithm.

Copy link
Author

Choose a reason for hiding this comment

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

I could not get butler to work/save parameters unless the variable in the yaml. I am not sure of the 'correct' way to get butler working.

Copy link
Member

Choose a reason for hiding this comment

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

Hm... that's not how butler is supposed to work. It looks like this should work if this is removed beacuse pT is initialized. If it doesn't, could you give a traceback?

Copy link
Author

Choose a reason for hiding this comment

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

I went back and removed the arg in myApp.yaml and it continued to work as expected. I am not sure what I was doing at the time of writing that...


# Update t for next run
newT = t + 1
butler.algorithms.set(key='pt', value=newT)
Copy link
Member

Choose a reason for hiding this comment

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

I think there's a typo here (pt vs pT). But maybe we could provide a longer name, maybe answers_received? That'd be a little easier to maintain.

Copy link
Author

Choose a reason for hiding this comment

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

I agree, a more descriptive name would be better.

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.

2 participants