fix faulty header of random search selector#70
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e647182267
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| seq = result.sequences | ||
| prompt_with_examples = Prompt(prompt.instruction, [seq[0][0]]) | ||
| seq = result.sequences[0] | ||
| examples = random.sample(list(seq), n_examples) |
There was a problem hiding this comment.
random.sample(list(seq), n_examples) raises ValueError whenever the sampled subsplit has fewer items than n_examples (e.g., small datasets where len(xs) < 5, or any run with config.n_exemplars larger than the subsample size). Because BaseTask.subsample can legitimately return fewer rows than requested, this turns exemplar selection into a hard runtime failure instead of returning a best-effort prompt.
Useful? React with 👍 / 👎.
finitearth
force-pushed
the
69-keywords-in-randomsearchselectorselect_exemplars
branch
from
e647182 to
de3b610
Compare
|
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #69 by updating RandomSearchSelector.select_exemplars() to accept an n_examples parameter, and adds regression/unit tests around exemplar selector behavior. It also updates the README coverage badge.
Changes:
- Add
n_examplesparameter toRandomSearchSelector.select_exemplars()and update selection logic to pick multiple exemplars. - Add new test suite for exemplar selectors, including regression coverage for the
n_exampleskeyword argument. - Update README coverage badge from 95% to 96%.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
promptolution/exemplar_selectors/random_search_selector.py |
Fix selector API to accept n_examples and sample multiple exemplars per trial. |
tests/exemplar_selectors/test_exemplar_selectors.py |
Add tests validating exemplar selection size, best-trial behavior, and regression for n_examples kwarg. |
README.md |
Update coverage badge percentage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| best_score = 0.0 | ||
| best_prompt = prompt | ||
|
|
There was a problem hiding this comment.
@finitearth I guess this is a valid point, but probably not the only place where we set it like that? not sure if we have to talk about optimization conventions in general throughout promptolution
| seq = result.sequences[0] | ||
| examples = random.sample(list(seq), n_examples) |
finitearth
deleted the
69-keywords-in-randomsearchselectorselect_exemplars
branch
This is a fix to issue #69, which flagged that we are missing a parameter for the Random Search selector to define how many examples should be searched. Added tests for exemplar selectors.