-
Notifications
You must be signed in to change notification settings - Fork 2
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
Include a cold start bandit test case #71
Conversation
ufc/bandit-flags-v1.json
Outdated
@@ -227,6 +227,31 @@ | |||
} | |||
], | |||
"totalShards": 10000 | |||
}, | |||
"cold_start_bandit": { |
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.
Flag in the UFC so that we can request an assignment from the cold-started bandit
@@ -0,0 +1,81 @@ | |||
{ | |||
"flag": "cold_start_bandit_flag", |
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.
The new test case! We check all three variations
@@ -286,7 +311,7 @@ | |||
"variationValue": "banner_bandit" | |||
} | |||
], | |||
"modelVersion": "v123" | |||
"modelVersion": "123" |
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.
Removing the v
to mirror the production change we're making. It doesn't make a material difference as long as this is consistent with bandit-models-v1.json
We want to test when a bandit has cold-started, e.g., no model parameters have been generated yet.
Note that the UFC is currently inconsistent with the name of the version of a cold-started bandit, with "
cold start
" used in the model parameters andnull
used in the UFC. We need to align on one to avoid errors.The plan is to align on committing the
v
prefix and using the"cold start"
placeholder as this is more SDK friendly.See all passing with
cold start
as the model version: https://github.com/Eppo-exp/sdk-test-data/actions/runs/11715905905/job/32633092353?pr=71Mixed Results when
null
is the model version: https://github.com/Eppo-exp/sdk-test-data/actions/runs/11715993935/job/32633330226?pr=71This PR updates the test data to match that plan.