Skip to content

Fix correctness bugs in the evolutionary optimisation core - #295

Merged
nicl-nno merged 6 commits into
mainfrom
fix/evolution-correctness
Aug 19, 2026
Merged

Fix correctness bugs in the evolutionary optimisation core#295
nicl-nno merged 6 commits into
mainfrom
fix/evolution-correctness

Conversation

@nicl-nno

@nicl-nno nicl-nno commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

What

A batch of correctness fixes for the evolutionary optimisation core. Each bug was first reproduced experimentally against main, then fixed, and each fix carries a regression test.

1. keep_n_best elitism could freeze evolution (elitism.py)

remain_n = len(new_population) - len(best_individuals) was never floored at zero. Once the archive reaches the population size, the next generation is composed of the same elite individuals verbatim, generation after generation — no error, no warning, the run just silently stops exploring. With an archive larger than the population the negative slice also let the population grow beyond pop_size (reproduced: 8 individuals in a population of 4).

Now at most len(new_population) - 1 elites are kept, so at least one offspring always survives elitism.

2. Extending the initial population silently cost one generation (gp_optimizer.py, generation_keeper.py)

_initial_population appends twice when the initial individuals are fewer than pop_size (initial_assumptions + extended_initial_assumptions), and each append incremented the generation counter that the stop condition reads. Any run that started from fewer assumptions than pop_size — the common case — executed num_of_generations - 1 real evolutionary steps. Reproduced end-to-end: 3/4 generations with extension vs 4/4 without; the history looks complete because the extension record masquerades as a generation.

GenerationKeeper.append now takes evolutionary_step: bool = True; the extension append passes False, so the archive is still updated but the generation counter and stagnation trackers are untouched. Both EvoGraphOptimizer and PopulationalRandomMutationOptimizer are covered.

3. One-point crossover's sink guard was a no-op for multi-root graphs (crossover.py)

first is not root_first compared a node against whatever graph.root_node returns — which is a list for graphs with zero or several roots, so the identity check was always true and the sink filter silently accepted every pair. Multi-root graphs reach this code: DEFAULT_DAG_RULES includes has_root but not has_one_root. The check now tests identity against each element of root_nodes().

4. A crossover that gave up short-circuited the retry budget and bred duplicates (crossover.py)

When the sink-filter search finds no valid pick, the crossover functions return the parents unchanged. Unchanged graphs trivially pass verification, so Crossover._crossover accepted them on the first outer attempt — collapsing max_num_of_operator_attempts independent draws into one — and wrapped them into fresh Individuals with null fitness, forcing a duplicate evaluation of already-evaluated graphs. The same waste existed before the sink filter whenever replace_subtrees rejected a swap by the depth guard.

_crossover now detects structurally unchanged offspring, spends the attempt budget on fresh random draws, and falls back to the original (already evaluated) individuals.

5. Individuals from _extend_population shared one mutable graph (populational_optimizer.py)

The base-class implementation passed choice(pop).graph by reference, so all extension individuals aliased a single graph object (reproduced: 5 individuals, 1 unique id()). Live for PopulationalRandomMutationOptimizer and any subclass that does not override the method; EvoGraphOptimizer overrides it and was not affected. Now deep-copies.

6. Crash in the stagnation-timeout stop condition (populational_optimizer.py)

With timeout=None and early_stopping_timeout=None the fallback early_stopping_timeout or self.timer.timeout still yields None, and the condition raises TypeError: '>=' not supported between 'float' and 'NoneType'. The adjacent iteration-based condition already guards against None; the timeout condition now does the same.

7. Selection treated an explicit pop_size=0 as "use default" (selection.py)

pop_size or self.parameters.pop_size — falsy-zero. Now if pop_size is not None.

Second commit — continued audit

8. are_subtrees_the_same counted every child pair as matched (gp_operators.py)

if (node, node2) or (node2, node) in match_set: parses as (node, node2) or ((node2, node) in match_set) — a non-empty tuple is always truthy, so the membership test never ran. equivalent_subtree therefore reported structurally different subtrees as equivalent, degrading one-point crossover into a random subtree exchange. Fixed the operator precedence.

9. The "mutation probability = 1.0" overrides were silently ineffective (gp_optimizer.py, agent_trainer.py)

Both EvoGraphOptimizer._extend_population and AgentTrainer.fit assigned mutation_prob = 1.0 to GraphRequirements — a stray attribute, because Mutation._will_mutation_be_applied reads self.parameters.mutation_prob from GPAlgorithmParameters. The extension of the initial population (which runs in nearly every optimization) was wasting ~20% of its capped attempts on guaranteed no-ops with the default 0.8 probability, and the trainer's documented "always mutate" mode never happened (reproduced: 0/1000 mutations applied with parameters.mutation_prob=0). Both sites now override the algorithm parameters and restore them afterwards.

10. validate_on_rollouts summed the action instead of the reward (agent_trainer.py)

TrajectoryStep is (individual, action, reward), but the code unpacked (_, reward, _) — binding the action, a string, so any non-empty trajectory raised TypeError on sum().

11. ExperienceBuffer.split inverted the split for tiny buffers (experience_buffer.py)

int(len * ratio) == 0 made the mask slice [-0:] cover the whole array, handing everything to train and nothing to validation — the opposite of intended.

Verification

  • every fix was reproduced as failing behaviour on main first (scripted, not by inspection);
  • regression tests added: test_generation_count.py, test_experience_buffer.py (new), plus cases in test_crossover.py, test_elitism.py, test_selection.py, test_generation_keeper.py, test_gp_operators.py — each fails on the unfixed code;
  • full test/unit/optimizers/ + test/unit/dag/ suite passes (285 passed; the pre-existing test/unit/adaptive MAB failures reproduce on main and are unrelated).

Related

The sink-filter hooks from #294 (can_be_sink / get_final_node) are still no-ops for FEDOT until its PipelineChangeAdvisor / PipelineOptNodeFactory override them — that needs a follow-up PR in the FEDOT repo; fixes 3 and 4 here make the GOLEM side behave correctly once that lands.

@nicl-nno

nicl-nno commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator Author

Second commit (3d7acba) — four more fixes from the continued audit, same method: each reproduced on the unfixed code first; each regression test fails before and passes after.

  1. are_subtrees_the_same counted every child pair as matched (gp_operators.py): (node, node2) or (node2, node) in match_set parses as (node, node2) or (...) — a non-empty tuple is always truthy, so the membership test never ran. equivalent_subtree paired structurally different subtrees and one-point crossover degraded into a random subtree exchange.
  2. The "mutation probability = 1.0" overrides were silently ineffective (gp_optimizer.py _extend_population, agent_trainer.py fit): the probability was assigned to GraphRequirements, but Mutation reads parameters.mutation_prob from GPAlgorithmParameters (reproduced: 0/1000 mutations applied). Both sites now override the algorithm parameters and restore them afterwards.
  3. validate_on_rollouts summed the action instead of the reward (agent_trainer.py): TrajectoryStep is (individual, action, reward) but was unpacked as (_, reward, _) — the action is a string, so any non-empty trajectory raised TypeError on sum().
  4. ExperienceBuffer.split inverted the split for tiny buffers (experience_buffer.py): int(len * ratio) == 0 made the mask slice [-0:] cover the whole array — train got everything, validation nothing.

With the second commit the suite is 285 passed (test/unit/optimizers/ + test/unit/dag/ + the new test_experience_buffer.py); the pre-existing test/unit/adaptive MAB failures reproduce on main and are unrelated to this PR.

@codecov-commenter

codecov-commenter commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.00000% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.71%. Comparing base (4f5ecf8) to head (05c14c4).

Files with missing lines Patch % Lines
golem/core/optimisers/adaptive/agent_trainer.py 0.00% 7 Missing ⚠️
...ore/optimisers/random/random_mutation_optimizer.py 0.00% 1 Missing ⚠️
golem/version.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #295      +/-   ##
==========================================
+ Coverage   72.06%   72.71%   +0.64%     
==========================================
  Files         142      142              
  Lines        8485     8450      -35     
==========================================
+ Hits         6115     6144      +29     
+ Misses       2370     2306      -64     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Seven fixes, each reproduced experimentally against main before fixing
and covered by a regression test:

* keep_n_best elitism froze evolution once the archive reached the
  population size (remain_n was never floored at zero; a larger archive
  also inflated the population via negative slicing). At most
  pop_size - 1 elites are kept now, so offspring always survive.

* Extending the initial population up to pop_size silently cost one
  evolutionary generation: both setup appends incremented the generation
  counter the stop condition reads. The extension append is now
  bookkeeping (evolutionary_step=False) - the archive is updated, the
  counters are not.

* one_point_crossover's sink guard compared a node to graph.root_node
  with `is not`, but root_node returns a list for multi-root graphs,
  so the guard silently accepted every pair. Identity is now checked
  against each element of root_nodes().

* A crossover that gave up (sink filter or depth guard) returned the
  parents unchanged; the unchanged copies trivially passed verification,
  short-circuiting the max_num_of_operator_attempts retry budget on the
  first draw and forcing duplicate evaluations of already-evaluated
  graphs wrapped in fresh Individuals with null fitness. Unchanged
  offspring now spend the attempt budget on fresh draws, and the
  original evaluated individuals are returned when nothing new is bred.

* The base _extend_population shared one mutable graph object between
  all extension individuals; it deep-copies now.

* The stagnation-timeout stop condition raised TypeError when both
  timeout and early_stopping_timeout are None; it is now guarded the
  same way as the iteration-based condition.

* Selection treated an explicit pop_size=0 as "use the default size"
  due to a falsy-zero `or`.
* are_subtrees_the_same: `(node, node2) or (node2, node) in match_set`
  parsed as `(node, node2) or ((node2, node) in match_set)` - a non-empty
  tuple is always truthy, so every child pair counted as matched and
  equivalent_subtree reported structurally different subtrees as
  equivalent, degrading one-point crossover into a random exchange.

* The "set mutation probability to 1.0" overrides in
  EvoGraphOptimizer._extend_population and AgentTrainer.fit assigned the
  probability to GraphRequirements, but Mutation reads it from the
  algorithm parameters - the override was silently ineffective. Both
  sites now override the parameters and restore them afterwards.

* AgentTrainer.validate_on_rollouts unpacked TrajectoryStep as
  (_, reward, _), which binds the action (a string) instead of the
  reward, so any non-empty trajectory raised TypeError on sum().

* ExperienceBuffer.split inverted the split for tiny buffers:
  int(len * ratio) == 0 made the mask slice [-0:] cover the whole
  array, handing everything to train and nothing to validation.

Each fix is covered by a regression test that fails on the unfixed code.
@nicl-nno
nicl-nno force-pushed the fix/evolution-correctness branch from b3b716d to 3d7acba Compare August 19, 2026 15:41
* python_requires >=3.10; classifiers and both CI matrices now cover
  3.10 through 3.14 (3.8 and 3.9 are dropped).

* numpy < 2 and scipy < 1.13 have no builds for Python 3.13+, so the
  old pins are kept via environment markers for python_version < '3.13'
  (where they still guard downstream compatibility) and replaced with
  numpy >= 2.1 / scipy >= 1.14.1 on newer interpreters. Same split for
  MarkupSafe (2.1.1 has no cp313+ wheels).

* The 'typing' PyPI backport is removed from requirements: it is the
  standard library since Python 3.5 and breaks installs on new versions.

* graph_viz: np.cross dropped support for 2-dimensional vectors in
  numpy 2.0; the edge-curvature code now computes the z-component of
  the 2D cross product explicitly.

Verified experimentally on a local Python 3.14.7 venv: a clean
`pip install .` resolves (numpy 2.5.2, scipy 1.18.0, pandas 3.0.5,
iOpt 0.2.22) and the optimizer/dag/adaptive unit suites pass 285/285,
same as on 3.10.
The 3.13 CI job died resolving dependencies: karateclub's transitive
gensim pin requires numpy < 2, which has no builds for Python 3.13+, so
pip backtracked across the whole graph until it hit an old Pillow sdist
whose setup.py crashes on new interpreters. karateclub is therefore
limited to python_version < '3.13' (the feather_graph context agent
already degrades gracefully without it), and the tests parametrized with
feather_graph skip when karateclub is absent.

Two more incompatibilities surfaced by running the full unit suite on a
local Python 3.14 venv with all extras installed:

* iOpt 0.2.22 still calls np.infty, removed in numpy 2.0, so IOptTuner
  cannot work on 3.13+ until iOpt catches up; its test cases skip when
  numpy lacks infty.

* test_load_mab asserted `loaded_mab.__eq__(mab)` - neither class
  defines __eq__, so the call always returned NotImplemented, which was
  truthy (with a warning) before Python 3.14 and is a TypeError in a
  boolean context since. The vacuous line is dropped; the field-wise
  asserts below it are the actual check.

The unit-build matrix also sets fail-fast: false, so one failing
interpreter no longer cancels the others.

With these changes the full test/unit suite passes on the local 3.14
venv with .[docs], .[profilers] and .[adaptive] installed:
473 passed, 7 skipped.
The 3.11/3.12 CI jobs showed that karateclub's unmaintained dependency
caps (gensim -> numpy < 2, pandas <= 1.3.5) only resolve into
installable wheels up to Python 3.10: on 3.11 pip backtracked to the
pandas 1.3.5 sdist, whose build fails on modern setuptools. The extra
is therefore limited to python_version < '3.11' - matching the versions
it was actually tested on before - and the feather_graph test skips
follow the runtime availability check, so they need no version logic.
* the Python-versions badge is now a static 3.10-3.14 badge: the
  pypi/pyversions one keeps showing the versions of the latest
  *released* wheel, which would misreport the supported range until
  0.5.0 ships (and lag behind again on every future change);
* the license badge carried a copy-pasted "Supported Python Versions"
  alt text;
* the pypi and python badges linked to their own image URLs instead of
  the project page.
@nicl-nno
nicl-nno merged commit 2ee6d82 into main Aug 19, 2026
8 checks passed
@nicl-nno
nicl-nno deleted the fix/evolution-correctness branch August 19, 2026 18:46
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