Skip to content

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!

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