Skip to content

Comments

Fix remove_cycles_dfs missing cycles due to wrong back-edge parent#111

Merged
HyperCodec merged 5 commits intodevfrom
copilot/sub-pr-106
Feb 23, 2026
Merged

Fix remove_cycles_dfs missing cycles due to wrong back-edge parent#111
HyperCodec merged 5 commits intodevfrom
copilot/sub-pr-106

Conversation

Copy link

Copilot AI commented Feb 20, 2026

remove_cycles would silently leave cycles intact after crossover. The colored DFS used prev: Option<NeuronLocation> to identify the back-edge, but HashMap's non-deterministic iteration order could cause prev to hold a grandparent (or earlier ancestor) rather than the true immediate parent. The guard self[prev].outputs.contains_key(&current) would then return false, and the cycle edge was never added to edges_to_remove.

Changes

  • remove_cycles_dfs: Replace prev: Option<NeuronLocation> parameter with an explicit path: &mut Vec<NeuronLocation> DFS stack. path.last() is guaranteed to be the immediate predecessor of current at the point a back-edge is detected, regardless of HashMap iteration order.
// before – prev could be a wrong ancestor
fn remove_cycles_dfs(..., prev: Option<NeuronLocation>, current: NeuronLocation) {
    if existing == 0 {
        let prev = prev.unwrap();
        if self[prev].outputs.contains_key(&current) {  // silently skipped when prev was wrong
            edges_to_remove.insert(Connection { from: prev, to: current });
        }
    }
}

// after – path.last() is always the true calling node
fn remove_cycles_dfs(..., path: &mut Vec<NeuronLocation>, current: NeuronLocation) {
    if existing == 0 {
        if let Some(&parent) = path.last() {
            edges_to_remove.insert(Connection { from: parent, to: current });
        }
    }
    visited.insert(current, 0);
    path.push(current);
    // ... recurse ...
    path.pop();
    visited.insert(current, 1);
}
  • Cleanup: Remove all temporary debug assertions added during investigation from split_connection, add_connection, and crossover.

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

Copilot AI and others added 2 commits February 20, 2026 19:40
Co-authored-by: HyperCodec <72839119+HyperCodec@users.noreply.github.com>
Co-authored-by: HyperCodec <72839119+HyperCodec@users.noreply.github.com>
The original remove_cycles_dfs used a `prev: Option<NeuronLocation>` parameter
to identify the back-edge when a cycle was detected. Due to HashMap iteration
non-determinism, `prev` could hold the wrong value (an ancestor instead of the
immediate parent), causing `self[prev].outputs.contains_key(&current)` to fail
silently and leaving cycles in the network.

Fix: replace `prev` with an explicit `path: &mut Vec<NeuronLocation>` stack.
`path.last()` is always the correct immediate predecessor of `current` in the
DFS call stack, regardless of HashMap iteration order.

Also remove all temporary debug assertions added during investigation.

Co-authored-by: HyperCodec <72839119+HyperCodec@users.noreply.github.com>
Copilot AI changed the title [WIP] Update to 1.0.0 with massive rewrite changes Fix remove_cycles_dfs missing cycles due to wrong back-edge parent Feb 23, 2026
@HyperCodec HyperCodec marked this pull request as ready for review February 23, 2026 13:48
Copy link
Owner

@HyperCodec HyperCodec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. idk if I really care about having all these extra debug test cases, but it's probably better to have them than not.

@HyperCodec HyperCodec merged commit 054ab38 into dev Feb 23, 2026
3 checks passed
@HyperCodec HyperCodec deleted the copilot/sub-pr-106 branch February 23, 2026 13:54
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