Improve the degree to which `SnarlDistanceIndex` actually implements `SnarlDecomposition` by adamnovak · Pull Request #247 · vgteam/libbdsg

adamnovak · 2026-10-15T22:43:03Z

In trying to generate code with a sensible design for that PR, without a bunch of workarounds for ways the new SnarlDecomposition implementation supposedly ought to be expected to differ from SnarlDistanceIndex, I generated a list of plausible-sounding bugs in how SnarlDistanceIndex implements SnarlDecomposition.

I then generated some tests for those bugs (forthcoming in that other PR), and here I have generated supposed fixes to pass those tests. This also involved generating some shortcuts that look like they might speed up PackedVector resizing, generating a proposal that there was a bug in PackedVector where you could reserve and then resize to see uninitialized data, and generating a fix.

I've repeatedly re-generated the comments here using different models and a bunch of Markdown skill files and sentence chooser scripts, which I also generated, but I have not had great luck so far in generating comments I actually want to read.

Once I put this up I'm going to review it, and then somehow attempt to climb out of the hole I have generated for myself, where I am three generated pull requests deep during an ostensible process of "review" of the first one.

An end-to-start traversal now lists them end-to-start, each flipped, and any other traversal lists them start-to-end, as the SnarlDecomposition interface requires. Co-authored-by: Adam Novak <anovak@soe.ucsc.edu> Assisted-by: Claude Code:claude-opus-5-5

…rsal The end bound is the chain's first node, traversed the way the chain goes, as the SnarlDecomposition interface allows a node only those two traversals. Going around the chain from its last child now reaches that same handle. Co-authored-by: Adam Novak <anovak@soe.ucsc.edu> Assisted-by: Claude Code:claude-opus-5-5

Going left from a traversal now gives each traversal from which going right gives it. Before, a chain node reached going left was oriented one way or the other depending on how nodes were packed into records. Co-authored-by: Adam Novak <anovak@soe.ucsc.edu> Assisted-by: Claude Code:claude-opus-5-5

A chain is start-start or end-end connected when its loop distance at that bound is finite. Those distances count turnarounds in trivial snarls and stop at snarls a walk cannot cross. Before, a chain took the turnarounds of every non-trivial snarl in it. Co-authored-by: Adam Novak <anovak@soe.ucsc.edu> Assisted-by: Claude Code:claude-opus-5-5

A snarl now reaches a tip through a child chain that reaches one from the side a walk enters it, or through a child chain's outer dead end. A chain reaches a tip when a walk along it, crossing and turning around where its snarls and loops allow, enters a snarl from a side that reaches a tip. Before, chains copied tip bits from their snarls before any snarl had them, and a child chain never gave its snarl a tip. Co-authored-by: Adam Novak <anovak@soe.ucsc.edu> Assisted-by: Claude Code:claude-opus-5-5

get_parent_traversal() now accepts a node that is a chain by itself as both bounds of its chain. Before, it looked for the node's parent snarl's bounds, and threw. Co-authored-by: Adam Novak <anovak@soe.ucsc.edu> Assisted-by: Claude Code:claude-opus-5-5

The SnarlDecomposition interface wants every parent that is not a chain traversed start-to-end. Before, a sentinel's parent snarl took a traversal that depended on which way the sentinel faced. Co-authored-by: Adam Novak <anovak@soe.ucsc.edu> Assisted-by: Claude Code:claude-opus-5-5

Going right from a traversal now gives each realizable traversal of the element entered that starts where the walk enters it, as the SnarlDecomposition interface says, and nothing from a traversal that ends at a tip. Before, it gave one traversal to the element's far end, realizable or not. Co-authored-by: Adam Novak <anovak@soe.ucsc.edu> Assisted-by: Claude Code:claude-opus-5-5

Co-authored-by: Adam Novak <anovak@soe.ucsc.edu> Assisted-by: Claude Code:claude-opus-5-5

Co-authored-by: Adam Novak <anovak@soe.ucsc.edu> Assisted-by: Claude Code:claude-fable-5-1

Co-authored-by: Adam Novak <anovak@soe.ucsc.edu> Assisted-by: Claude Code:claude-sonnet-5-5

adamnovak

I've improved some comments but also part of the approach is wrong.

adamnovak · 2026-10-16T15:46:12Z

+    // Return the snarl of the sentinel, traversed start-to-end whichever way
+    // the sentinel faces.


Suggested change

// Return the snarl of the sentinel, traversed start-to-end whichever way

// the sentinel faces.

// If the child is a sentinel of a snarl, just return the snarl, start-to-end

adamnovak · 2026-10-16T15:48:56Z

+        // Traverse the bound node start-to-end unless rev_in_parent is set.
+        // The end bound of a looping chain is the first node of the chain.


Suggested change

// Traverse the bound node start-to-end unless rev_in_parent is set.

// The end bound of a looping chain is the first node of the chain.

adamnovak · 2026-10-16T15:58:18Z

 }

-bool SnarlDistanceIndex::follow_net_edges_impl(const net_handle_t& here, const handlegraph::HandleGraph* graph, bool go_left, const std::function<bool(const net_handle_t&)>& iteratee) const {
+bool SnarlDistanceIndex::follow_net_edges_impl(const net_handle_t& here, const handlegraph::HandleGraph* graph, bool go_left, const std::function<bool(const net_handle_t&)>& each_reached) const {


We should not rename this.

Suggested change

bool SnarlDistanceIndex::follow_net_edges_impl(const net_handle_t& here, const handlegraph::HandleGraph* graph, bool go_left, const std::function<bool(const net_handle_t&)>& each_reached) const {

bool SnarlDistanceIndex::follow_net_edges_impl(const net_handle_t& here, const handlegraph::HandleGraph* graph, bool go_left, const std::function<bool(const net_handle_t&)>& iteratee) const {

adamnovak · 2026-10-16T15:59:17Z

+        // Find the traversals to the left by walking right from flip(here) and
+        // flipping each result. This works because a walk taken backward is
+        // still a walk.


Suggested change

// Find the traversals to the left by walking right from flip(here) and

// flipping each result. This works because a walk taken backward is

// still a walk.

// Implement going left using going right

adamnovak · 2026-10-16T16:34:05Z

+            // Return the first node of the chain, traversed forward along the
+            // chain. Going right from the last node of a looping chain enters
+            // that node.


I think the original comment was righter.

Suggested change

// Return the first node of the chain, traversed forward along the

// chain. Going right from the last node of a looping chain enters

// that node.

//IF we are going right from the last node of a looping chain, then

//return the first node going in

adamnovak · 2026-10-16T16:35:20Z

+                    // Mark the start of the chain as connected to itself when
+                    // forward_loops[0] is finite, and the end when
+                    // backward_loops.back() is finite. A walk entering at that
+                    // bound can then turn around and leave through it.


Suggested change

// Mark the start of the chain as connected to itself when

// forward_loops[0] is finite, and the end when

// backward_loops.back() is finite. A walk entering at that

// bound can then turn around and leave through it.

// Propagate connectivity from loop distances to connectivity flags

adamnovak · 2026-10-16T16:37:41Z

+                    // Mark the start of the chain as connected to itself when
+                    // forward_loops[0] is finite, and the end when
+                    // backward_loops.back() is finite. A walk entering at that
+                    // bound can then turn around and leave through it.
+                    if (!temp_chain_record.forward_loops.empty() &&
+                        temp_chain_record.forward_loops.front() != std::numeric_limits<size_t>::max()) {
+                        chain_record_constructor.set_start_start_connected();
+                    }
+                    if (!temp_chain_record.backward_loops.empty() &&
+                        temp_chain_record.backward_loops.back() != std::numeric_limits<size_t>::max()) {
+                        chain_record_constructor.set_end_end_connected();
+                    }


How are the loop distances (which depend on child connectivity) filled in if the child connectivity isn't filled in yet?

adamnovak · 2026-10-16T16:45:34Z

+    /* Set the start-tip, end-tip and tip-tip flags of each snarl and chain.
+     * The recursion in snarl_tips and chain_tips computes the flags of each
+     * child before those of its parent.
+     */
+    for (size_t temp_index_i = 0 ; temp_index_i < temporary_indexes.size() ; temp_index_i++) {
+        const TemporaryDistanceIndex* temp_index = temporary_indexes[temp_index_i];
+        const size_t inf = std::numeric_limits<size_t>::max();
+
+        // The tip flags of a snarl or chain. Each member says whether walks
+        // from one place can end at a tip. The places are the start, the end
+        // and a tip.
+        struct Tips {
+            bool from_start = false;
+            bool from_end = false;
+            bool from_tip = false;
+        };
+        // The tip flags of each snarl and chain computed so far, by record.
+        std::map<temp_record_ref_t, Tips> tips_found;
+        std::function<Tips(const temp_record_ref_t&)> snarl_tips;
+        std::function<Tips(const temp_record_ref_t&)> chain_tips;
+
+        snarl_tips = [&](const temp_record_ref_t& snarl_ref) -> Tips {
+            auto found = tips_found.find(snarl_ref);
+            if (found != tips_found.end()) {
+                return found->second;
+            }
+            const auto& snarl = temp_index->get_snarl(snarl_ref);
+            Tips tips;
+            if (!snarl.is_trivial && !snarl.is_simple && !snarl.is_root_snarl) {
+                size_t offset = record_to_offset.at(make_pair(temp_index_i, snarl_ref));
+                // Start from the tip-tip flag that the record already holds.
+                tips.from_tip = SnarlTreeRecord(offset, &snarl_tree_records).is_tip_tip_connected();
+                for (const temp_record_ref_t& child : snarl.children) {
+                    if (child.first == TEMP_NODE) {
+                        const auto& node = temp_index->get_node(child);
+                        // Find a tip from the start of the snarl when the
+                        // distance from the start to either side of a tip node
+                        // is finite. Do the same for the end.
+                        if (node.is_tip) {
+                            tips.from_start |= node.distance_left_start != inf || node.distance_right_start != inf;
+                            tips.from_end |= node.distance_left_end != inf || node.distance_right_end != inf;
+                        }
+                        continue;
+                    }
+                    const auto& chain = temp_index->get_chain(child);
+                    Tips child_tips = chain_tips(child);
+                    // Find a tip through a child chain. The distance_left_*
+                    // and distance_right_* members give the distances from the
+                    // bounds of the snarl to the left and right sides of the
+                    // chain. A walk entering on the left side starts at the
+                    // chain start, and on the right side at the chain end. A
+                    // walk that crosses the chain ends at a tip when the bound
+                    // node on the far side is a tip node.
+                    bool start_dead = temp_index->get_node(chain.children.front()).is_tip;
+                    bool end_dead = temp_index->get_node(chain.children.back()).is_tip;
+                    bool crossable = chain.min_length != inf;
+                    bool from_left = child_tips.from_start || (crossable && end_dead);
+                    bool from_right = child_tips.from_end || (crossable && start_dead);
+                    tips.from_start |= (chain.distance_left_start != inf && from_left) ||
+                                       (chain.distance_right_start != inf && from_right);
+                    tips.from_end |= (chain.distance_left_end != inf && from_left) ||
+                                     (chain.distance_right_end != inf && from_right);
+                    tips.from_tip |= child_tips.from_tip;
+                }
+            }
+            tips_found[snarl_ref] = tips;
+            return tips;
+        };
+
+        chain_tips = [&](const temp_record_ref_t& chain_ref) -> Tips {
+            auto found = tips_found.find(chain_ref);
+            if (found != tips_found.end()) {
+                return found->second;
+            }
+            const auto& chain = temp_index->get_chain(chain_ref);
+            Tips tips;
+            if (!chain.is_trivial) {
+                // The snarl after each node of the chain, indexed like
+                // forward_loops and backward_loops. snarl_after[i] is the
+                // snarl between node i and node i + 1, or nullptr for a
+                // trivial snarl and for the last node.
+                vector<const temp_record_ref_t*> snarl_after;
+                for (const temp_record_ref_t& child : chain.children) {
+                    if (child.first == TEMP_NODE) {
+                        snarl_after.push_back(nullptr);
+                    } else {
+                        snarl_after.back() = &child;
+                    }
+                }
+                size_t node_count = snarl_after.size();
+                // Return whether a walk from one of the given states can end
+                // at a tip in a snarl of the chain. A state (node_i, forward)
+                // is a walk leaving node_i forward along the chain if forward
+                // is true, and in reverse along it otherwise.
+                auto reaches_tip = [&](const vector<pair<size_t, bool>>& starts) {
+                    // A flag for each state that is true once the state is
+                    // queued, at index 2 * node_i + forward.
+                    vector<bool> seen(2 * node_count, false);
+                    // The stack of states that are waiting to be expanded.
+                    vector<pair<size_t, bool>> to_visit;
+                    // Add the state (node_i, forward) to to_visit, unless it
+                    // was queued before.
+                    auto visit = [&](size_t node_i, bool forward) {
+                        if (!seen[2 * node_i + forward]) {
+                            seen[2 * node_i + forward] = true;
+                            to_visit.emplace_back(node_i, forward);
+                        }
+                    };
+                    for (const auto& start : starts) {
+                        visit(start.first, start.second);
+                    }
+                    while (!to_visit.empty()) {
+                        auto [node_i, forward] = to_visit.back();
+                        to_visit.pop_back();
+                        if (forward ? node_i + 1 >= node_count : node_i == 0) {
+                            // Skip a state whose walk leaves the chain.
+                            continue;
+                        }
+                        // gap_i is the index in snarl_after of the snarl that
+                        // the walk crosses next, and across is the node on its
+                        // far side.
+                        size_t gap_i = forward ? node_i : node_i - 1;
+                        size_t across = forward ? node_i + 1 : node_i - 1;
+                        if (snarl_after[gap_i] == nullptr) {
+                            // Cross a trivial snarl to the next node. Turn
+                            // around at node_i when its entry in forward_loops
+                            // or backward_loops is 0, because the side of
+                            // node_i facing the walk then has an edge back to
+                            // itself.
+                            visit(across, forward);
+                            const vector<size_t>& loops = forward ? chain.forward_loops : chain.backward_loops;
+                            if (node_i < loops.size() && loops[node_i] == 0) {
+                                visit(node_i, !forward);
+                            }
+                            continue;
+                        }
+                        // Cross the snarl when min_length is finite. Turn
+                        // around in it when a walk entering at this bound can
+                        // leave at the same bound. Report a tip when a walk
+                        // entering at this bound can end at a tip.
+                        const auto& snarl = temp_index->get_snarl(*snarl_after[gap_i]);
+                        Tips snarl_tip = snarl_tips(*snarl_after[gap_i]);
+                        if (snarl.min_length != inf) {
+                            visit(across, forward);
+                        }
+                        if ((forward ? snarl.distance_start_start : snarl.distance_end_end) != inf) {
+                            visit(node_i, !forward);
+                        }
+                        if (forward ? snarl_tip.from_start : snarl_tip.from_end) {
+                            return true;
+                        }
+                    }
+                    return false;
+                };
+                // Start a walk from the start of the chain at node 0, going
+                // forward along the chain. Start a walk from its end at the
+                // last node, going in reverse along it.
+                if (node_count > 0) {
+                    tips.from_start = reaches_tip({{0, true}});
+                    tips.from_end = reaches_tip({{node_count - 1, false}});
+                }
+                // Start walks at the tips of each snarl. A walk taken backward
+                // is still a walk. A walk from a tip can therefore leave the
+                // snarl by its start, going in reverse along the chain, when a
+                // walk from the start can end at that tip. The same holds for
+                // the end, going forward along the chain.
+                vector<pair<size_t, bool>> from_tips;
+                for (size_t gap_i = 0 ; gap_i + 1 < node_count ; gap_i++) {
+                    if (snarl_after[gap_i] == nullptr) {
+                        continue;
+                    }
+                    Tips snarl_tip = snarl_tips(*snarl_after[gap_i]);
+                    tips.from_tip |= snarl_tip.from_tip;
+                    if (snarl_tip.from_start) {
+                        from_tips.emplace_back(gap_i, false);
+                    }
+                    if (snarl_tip.from_end) {
+                        from_tips.emplace_back(gap_i + 1, true);
+                    }
+                }
+                tips.from_tip |= !from_tips.empty() && reaches_tip(from_tips);
+            }
+            tips_found[chain_ref] = tips;
+            return tips;
+        };
+
+        // Set the flags of each nontrivial chain, and of each snarl that is
+        // neither trivial, simple nor a root snarl.
+        auto set_tips = [&](const temp_record_ref_t& ref, const Tips& tips) {
+            SnarlTreeRecordWriter record(record_to_offset.at(make_pair(temp_index_i, ref)), &snarl_tree_records);
+            if (tips.from_start) {
+                record.set_start_tip_connected();
+            }
+            if (tips.from_end) {
+                record.set_end_tip_connected();
+            }
+            if (tips.from_tip) {
+                record.set_tip_tip_connected();
+            }
+        };
+        for (size_t chain_i = 0 ; chain_i < temp_index->temp_chain_records.size() ; chain_i++) {
+            temp_record_ref_t chain_ref = make_pair(TEMP_CHAIN, chain_i);
+            if (!temp_index->get_chain(chain_ref).is_trivial) {
+                set_tips(chain_ref, chain_tips(chain_ref));
+            }
+        }
+        for (size_t snarl_i = 0 ; snarl_i < temp_index->temp_snarl_records.size() ; snarl_i++) {
+            temp_record_ref_t snarl_ref = make_pair(TEMP_SNARL, snarl_i);
+            const auto& snarl = temp_index->get_snarl(snarl_ref);
+            if (!snarl.is_trivial && !snarl.is_simple && !snarl.is_root_snarl) {
+                set_tips(snarl_ref, snarl_tips(snarl_ref));
+            }
+        }
+    }


We should not be doing our own new call-stack-based mutual recursion down the snarl tree when we just got through with a stack-on-the-heap-based traversal of the snarl tree. For one thing, I'm not sure we have a guarantee that the snarl tree is shallow enough to fit on the call stack. For another, we just did this!

If we need to fill the connectivity bits in as we return back up the tree, we should adjust the traversal we just did so that we can do some work (or even maybe all the work?) for the parent after the children.

But if the loop distances are already filled in, to be translated to connectivity information, above, then there's already a pass that goes in the right bottom-to-top order to fill in the connectivity information, because otherwise the loop distances couldn't be right. We should be able to easily do the connectivity tracking there too.

The temporary chain and snarl records now carry start-tip, end-tip and tip-tip flags, which the builder's bottom-up pass fills in, and the record writer copies them. This replaces the separate recursive pass over the snarl tree. An end-to-start chain traversal now visits its collected children in reverse, flipping each, and other traversals present their children directly. follow_net_edges_impl() keeps its iteratee parameter and filters through visit_reachable_traversals. Co-authored-by: Adam Novak <anovak@soe.ucsc.edu> Assisted-by: Claude Code:claude-opus-5-5

Co-authored-by: Adam Novak <anovak@soe.ucsc.edu> Assisted-by: Claude Code:claude-opus-5-5

adamnovak

This is better. I have some comments to replace and we need to mention in the header that we do this weird thing with always showing the boundary traversal of a circular chain as the first child.

adamnovak · 2026-10-16T21:22:40Z

+    // Keep the existing entries where they are when the width stays the same or grows.
+    // int_vector::resize() keeps the old bits as a prefix of the reallocated word array, and
+    // expand_width() rewrites the entries in place from the last one down, so neither one
+    // needs the element-by-element copy that narrowing takes. Entries past the old size are
+    // left uninitialized either way, as they also are by the copy, since int_vector::resize()
+    // is a plain realloc.


Suggested change

// Keep the existing entries where they are when the width stays the same or grows.

// int_vector::resize() keeps the old bits as a prefix of the reallocated word array, and

// expand_width() rewrites the entries in place from the last one down, so neither one

// needs the element-by-element copy that narrowing takes. Entries past the old size are

// left uninitialized either way, as they also are by the copy, since int_vector::resize()

// is a plain realloc.

// Take some shortcuts when possible

adamnovak · 2026-10-16T21:24:27Z

+    // Clear whole words instead of going through the element accessor. int_vector stores entry
+    // i at bit i * width() of data(), lowest bits first, so the entries before begin occupy the
+    // low begin_bit bits and everything above them can be cleared. The memset also reaches the
+    // unused bits of the last word, past the end of the vector. sdsl zeroes those bits itself
+    // whenever it reallocates, so clearing them changes nothing.


Suggested change

// Clear whole words instead of going through the element accessor. int_vector stores entry

// i at bit i * width() of data(), lowest bits first, so the entries before begin occupy the

// low begin_bit bits and everything above them can be cleared. The memset also reaches the

// unused bits of the last word, past the end of the vector. sdsl zeroes those bits itself

// whenever it reallocates, so clearing them changes nothing.

// Clear whole words instead of going through the element accessor. It's OK to also clear the trailing bits in the last word that aren't used in any entries.

adamnovak · 2026-10-16T21:26:45Z

+        // Every entry at index >= filled is zero after this, since reserve() leaves the
+        // entries past the old capacity uninitialized and entries dropped by a shrink keep
+        // their old values.


Suggested change

// Every entry at index >= filled is zero after this, since reserve() leaves the

// entries past the old capacity uninitialized and entries dropped by a shrink keep

// their old values.

// We need to zero from filled on up, because the memory there might contain old entries that were resized away.

adamnovak · 2026-10-16T21:28:31Z

+        //For looping chains, the boundary node is both the first and the last child, and has one handle
+        //for each orientation. Going right from the last child gives that handle, and going right from it
+        //again goes on around the chain, so a walk that should stop at the end must compare handles.


Suggested change

//For looping chains, the boundary node is both the first and the last child, and has one handle

//for each orientation. Going right from the last child gives that handle, and going right from it

//again goes on around the chain, so a walk that should stop at the end must compare handles.

//For looping chains, you can't walk out of the chain; you go around the loop instead.

adamnovak · 2026-10-16T21:34:21Z

+            // ChainRecord visits the children from the chain's start to its
+            // end, oriented forward along the chain.


Suggested change

// ChainRecord visits the children from the chain's start to its

// end, oriented forward along the chain.

// A start-to-end chain visits the children from the chain's start to its

// end, oriented forward along the chain.

adamnovak · 2026-10-16T21:35:34Z

+            // The SnarlDecomposition interface wants each child of any other
+            // traversal start-to-end.


Suggested change

// The SnarlDecomposition interface wants each child of any other

// traversal start-to-end.

// A chain in reversing or tip-to-tip orientations, or anything other than a chain, shows all the children start-to-end in arbitrary order.

adamnovak · 2026-10-16T21:36:06Z

+        // An end-to-start traversal visits the children in reverse order, each
+        // flipped, so collect them first.


Suggested change

// An end-to-start traversal visits the children in reverse order, each

// flipped, so collect them first.

// A chain in end-to-start orientation visits the children in reverse order, each

// flipped, so collect them first.

adamnovak · 2026-10-16T21:39:51Z

+        if (children.empty()) {
+            return true;
+        }
+        // In a looping chain the first child is both bounds, so it comes first.


This would be better explained as

Suggested change

// In a looping chain the first child is both bounds, so it comes first.

// In a looping chain the first child is both bounds, so it comes first even in end-to-start order

But also, the interface doesn't really say that we would show the one bound first no matter which way we look at a looping chain. It sort of makes sense, and might beat showing it twice or starting somewhere other than the bound for the reverse traversal, but it's weird and might need to be mentioned in the SnarlDistanceIndex header.

adamnovak · 2026-10-16T21:41:49Z

+    // The code below names each element the walk enters with a start-to-end
+    // or end-to-start traversal of it. Only that traversal's start endpoint
+    // matters: it is where the walk arrives. We use this function to
+    // enumerate all the actually possible traversals that start there.


Suggested change

// The code below names each element the walk enters with a start-to-end

// or end-to-start traversal of it. Only that traversal's start endpoint

// matters: it is where the walk arrives. We use this function to

// enumerate all the actually possible traversals that start there.

// We use this function to enumerate all the actually possible traversals that start at some endpoint of a child.

// We (ab)use a net_handle_t to represent both the child and the endpoint to start at; we ignore where the handle's traversal type is supposed to end.

The header now says that a looping chain lists its boundary node first, whichever way the chain is traversed. Co-authored-by: Adam Novak <anovak@soe.ucsc.edu> Assisted-by: Claude Code:claude-opus-5-5

adamnovak

I think this is ready now. I've basically rewritten all the comments myself.

Automated Commit and others added 14 commits October 14, 2026 20:05

Explain the snarl decomposition fixes in the decomposition's own terms

90c3c3c

Co-authored-by: Adam Novak <anovak@soe.ucsc.edu> Assisted-by: Claude Code:claude-opus-5-5

Say what the tip pass's vectors hold and what each walk step checks

d625ba5

Co-authored-by: Adam Novak <anovak@soe.ucsc.edu> Assisted-by: Claude Code:claude-opus-5-5

Rewrite the snarl decomposition fix comments to read on one pass

47f03c2

Co-authored-by: Adam Novak <anovak@soe.ucsc.edu> Assisted-by: Claude Code:claude-opus-5-5

Rewrite the snarl decomposition fix comments for a human reader

ca6b30f

Co-authored-by: Adam Novak <anovak@soe.ucsc.edu> Assisted-by: Claude Code:claude-fable-5-1

Explain why repack() and zero_from() may skip the element-wise copy

8f97707

Co-authored-by: Adam Novak <anovak@soe.ucsc.edu> Assisted-by: Claude Code:claude-fable-5-1

Zero entries exposed by PackedVector::resize() growing within capacity

83a6f40

Co-authored-by: Adam Novak <anovak@soe.ucsc.edu> Assisted-by: Claude Code:claude-sonnet-5-5

adamnovak commented Oct 16, 2026

View reviewed changes

Automated Commit and others added 2 commits October 16, 2026 13:37

Drop the TODO about oversized snarls lacking tip flags

c672c32

Co-authored-by: Adam Novak <anovak@soe.ucsc.edu> Assisted-by: Claude Code:claude-opus-5-5

adamnovak commented Oct 16, 2026

View reviewed changes

Apply review comment suggestions and note the looping chain listing

ded22a7

The header now says that a looping chain lists its boundary node first, whichever way the chain is traversed. Co-authored-by: Adam Novak <anovak@soe.ucsc.edu> Assisted-by: Claude Code:claude-opus-5-5

adamnovak commented Oct 16, 2026

View reviewed changes

Comment thread bdsg/include/bdsg/snarl_distance_index.hpp Resolved

Add note about least-weird decision

ff6c2dd

adamnovak commented Oct 16, 2026

View reviewed changes

adamnovak merged commit 485b32b into master Oct 16, 2026
1 check passed

		// Return the snarl of the sentinel, traversed start-to-end whichever way
		// the sentinel faces.

	// Return the snarl of the sentinel, traversed start-to-end whichever way
	// the sentinel faces.
	// If the child is a sentinel of a snarl, just return the snarl, start-to-end

		// Traverse the bound node start-to-end unless rev_in_parent is set.
		// The end bound of a looping chain is the first node of the chain.

	bool SnarlDistanceIndex::follow_net_edges_impl(const net_handle_t& here, const handlegraph::HandleGraph* graph, bool go_left, const std::function<bool(const net_handle_t&)>& each_reached) const {
	bool SnarlDistanceIndex::follow_net_edges_impl(const net_handle_t& here, const handlegraph::HandleGraph* graph, bool go_left, const std::function<bool(const net_handle_t&)>& iteratee) const {

		// ChainRecord visits the children from the chain's start to its
		// end, oriented forward along the chain.

		// The SnarlDecomposition interface wants each child of any other
		// traversal start-to-end.

	// The SnarlDecomposition interface wants each child of any other
	// traversal start-to-end.
	// A chain in reversing or tip-to-tip orientations, or anything other than a chain, shows all the children start-to-end in arbitrary order.

		// An end-to-start traversal visits the children in reverse order, each
		// flipped, so collect them first.

	// In a looping chain the first child is both bounds, so it comes first.
	// In a looping chain the first child is both bounds, so it comes first even in end-to-start order

Conversation

adamnovak commented Oct 15, 2026

Uh oh!

adamnovak left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamnovak left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

adamnovak left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant