Repository navigation
Conversation
…hmicsuperintelligence#516) _serialize_config built the worker config dict from a hand-written list of top-level Config fields. Fields missing from that list silently fell back to their defaults inside worker processes, so enforce_evolve_blocks and diff_pattern set in the config had no effect during a run. - Build the top-level part of the worker dict from dataclasses.fields(Config) so new fields reach workers automatically - Send evolution_trace as a dict and rebuild it in _worker_init, like the other nested config sections - Add a regression test that round-trips a config with non-default values through _serialize_config -> _worker_init and checks every field Fixes algorithmicsuperintelligence#516
This branch has not been deployed
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.
Fixes #516
Problem:
ProcessParallelController._serialize_configbuilt the dict sent to worker processes from a hand-written list of top-levelConfigfields._worker_initrebuilds the worker's config withConfig(**dict), so any field missing from that list silently fell back to its default inside workers.Two of the missing fields are read only in workers, so setting them in the config had no effect during a run:
enforce_evolve_blocks: true: edits outsideEVOLVE-BLOCK-START/ENDwere kept and evaluateddiff_pattern: replies were parsed with the default regex, so every iteration failed with "No valid diffs found in response"The existing worker tests patch
_worker_configdirectly, so the serialize →_worker_initround trip was never exercised.What:
dataclasses.fields(Config)instead of a hand-written list, so new fields reach workers automatically. The fields come from theConfigclass rather than the instance, so a user-definedConfigsubclass with extra fields still initialises workers.evolution_traceas a dict and rebuild it in_worker_init, likeprompt,databaseandevaluator.tests/test_worker_config_serialization.py: round-trips a config with a non-default value for every top-level field through_serialize_config→_worker_initand asserts each one matches. It also fails if a future field is added without a non-default value in the test.The other six previously dropped fields (
logger_prefix,evolution_trace,early_stopping_*,max_tasks_per_child) are only read in the main process, so forwarding them changes nothing in workers.Red -> green evidence:
On
main, the new test fails on exactly the eight dropped fields:With this PR:
Ran 3 tests ... OK.Testing:
mainplus the 3 new tests); per-test results are otherwise identical tomainforkandspawn, 3 workers:enforce_evolve_blocks: true: outside edits kept in 6/6 children onmain, 0/6 with this PRdiff_pattern: 6/6 iterations fail onmain, 0/6 with this PRmainmainNotes:
Open PR #480 adds these two keys to
_serialize_configas part of a much larger feature; this PR is a standalone fix with a regression test.