Fix degeneracy order and remove more clones #1

michaelsproul · 2023-10-31T05:19:46Z

Fix degeneracy_order. It was doing the indexing-after-deleting thing, which was not correct. I think the tests were passing because the degeneracy order doesn't affect correctness (just speed).
Remove some unnecessary clones in bron_kerbosch. We could use either HashTrieSet or Vec for the neighbourhoods. I'm not sure which is better, they seem about the same speed in testing, so using Vec for now.
Tweak the asserts in the attestation_pairwise_overlapping test so that they pass. Another configuration that is feasible (but blows up badly) is num_validators = 15 * spec.target_committee_size with step_size = 1.
Correct typo in name of compute_neighbourhood function.

michaelsproul · 2023-10-31T05:20:17Z

beacon_node/operation_pool/src/bron_kerbosch.rs

-        }
-    }
-    o
+    v.sort_unstable_by_key(|i| neighbourhoods[*i].len());


I'm pretty sure this is what was intended, but would appreciate a double-check from you @GeemoCandama

Yes that should be better. I don't know how I overlooked this before. I hadn't changed this from the Satalia code.

michaelsproul added 2 commits October 31, 2023 16:18

Fix degeneracy order and remove more clones

e1a01e3

Back to vecs

c53d236

michaelsproul commented Oct 31, 2023

View reviewed changes

GeemoCandama merged commit 74d0a15 into GeemoCandama:bron_kerbosch_attestation_aggregation Oct 31, 2023

michaelsproul deleted the bron_kerbosch_attestation_aggregation branch October 31, 2023 06:31

Provide feedback