fix: make 'best' parent-selection mode reachable from CLI and sort by highest score#32
Open
mvanhorn wants to merge 1 commit into
Open
fix: make 'best' parent-selection mode reachable from CLI and sort by highest score#32mvanhorn wants to merge 1 commit into
mvanhorn wants to merge 1 commit into
Conversation
… highest score Fixes jennyzzt#31
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix the argparse
choiceslist inDGM_outer.pyby adding the missing comma so it reads['random', 'score_prop', 'score_child_prop', 'best']. Inchoose_selfimproves(), addreverse=Trueto thesorted(...)call in theelif method == 'best':branch so the highestaccuracy_scorecandidates are chosen first. Add a new root-leveltest_dgm_outer.py(mirroring the existing root-leveltest_swebench.pyconvention; the repo has apytest.ini) with a unit test that callschoose_selfimproves(..., method='best')against synthetic candidate metadata (three fake commits with distinct scores, using tmp_path-backed metadata or monkeypatched loading as the function's data access requires) and asserts the top-scoring parent is selected first, plus a test that the argparse parser accepts bothbestandscore_child_prop.Why this matters
DGM_outer.pyhas two related bugs in thebestparent-selection path. First, the argparsechoiceslist for--choose_selfimproves_methodis written as['random', 'score_prop', 'score_child_prop' 'best']- the missing comma makes Python concatenate the adjacent string literals into'score_child_propbest', sobest(and an explicitly passedscore_child_prop) are rejected by the CLI. Second, themethod == 'best'branch inchoose_selfimproves()sorts candidates byaccuracy_scorein ascending order and takes the first N, so it selects the lowest-scoring parents instead of the highest-scoring ones. Because parent selection is the search pressure in DGM, an invertedbeststrategy silently corrupts any experiment comparing selection strategies.See #31.
Testing
choose_selfimproves(..., method='best')with three synthetic candidates scoring 0.2/0.5/0.9 and selfimprove_size=1 returns the 0.9-score commit. - selfimprove_size=2 returns the top two commits in descending score order. - selfimprove_size larger than the candidate count returns all candidates without error. - CLI parser accepts--choose_selfimproves_method bestand--choose_selfimproves_method score_child_prop; an invalid value (e.g.Fixes #31