Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement biparition_graph_mst and bipartition_tree functions #572

Open
wants to merge 46 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
1cdf76a
Draft bipartition_tree implementation
InnovativeInventor Jan 12, 2022
20281b0
Working bipartition tree impl
InnovativeInventor Jan 12, 2022
f85eb52
Release GIL during most of balanced_edge finding code
InnovativeInventor Jan 13, 2022
79b3d94
Ensure that unused vars get gc'ed on each loop
InnovativeInventor Jan 15, 2022
d5d464b
Lint with cargo fmt
InnovativeInventor Mar 19, 2022
e7409a5
Update to using built-in retworkx macros
InnovativeInventor May 16, 2022
a45c147
Take advantage of built-in retworkx macros
InnovativeInventor May 16, 2022
4d93f0e
Use mem::take to save on memory allocs
InnovativeInventor May 17, 2022
f01201c
Revert "Use mem::take to save on memory allocs"
InnovativeInventor May 17, 2022
fdbeaaf
Fix unnecessary clones; address clippy warnings
InnovativeInventor May 17, 2022
517d697
Remove unnecessary dead/commented out code
InnovativeInventor May 17, 2022
2bd4c33
Reduce number of memory allocs
InnovativeInventor May 18, 2022
658ffa5
Remove unused import
InnovativeInventor May 18, 2022
a360695
Remove no longer relevant todos
InnovativeInventor May 18, 2022
c5d76cb
Switch to using HashSet for seen_nodes tracker
InnovativeInventor May 18, 2022
6643195
Touch up comments to make more sense
InnovativeInventor May 18, 2022
90d5ba3
Lint with cargo fmt
InnovativeInventor May 18, 2022
b522daa
Rename functions to biparition_tree and bipartition_graph; update docs
InnovativeInventor May 23, 2022
1e6290d
Make docstrings more descriptive and fix renaming issue
InnovativeInventor May 23, 2022
c283355
Add release notes for bipartition_tree and bipartition_graph
InnovativeInventor May 23, 2022
cbaed09
Add bipartition_graph and bipartition_tree to API section of docs
InnovativeInventor May 23, 2022
41e503a
Shorten description of weight_fn by pointing to minimum_spanning_tree…
InnovativeInventor May 23, 2022
da1e8a7
Fix end with blank line linting issue
InnovativeInventor May 24, 2022
99e5257
Add bipartition tests
InnovativeInventor May 24, 2022
db87924
Fix indent issues in retworkx bipartition docstrings
InnovativeInventor May 24, 2022
f6d807c
Switch to using hashbrown's HashSet impl
InnovativeInventor May 27, 2022
46404f3
Make tests deterministic
InnovativeInventor May 27, 2022
1ea5cd1
Reorder imports as per cargo fmt
InnovativeInventor May 27, 2022
79b5e17
Wrap in rst Python code block
InnovativeInventor May 27, 2022
4e00b9b
Switch to passing by value for mst
InnovativeInventor May 27, 2022
5ba7cc9
Make test name more accurate
InnovativeInventor May 27, 2022
3685720
Handle holes in graph node indices
InnovativeInventor May 27, 2022
8e513f7
Revert "Switch to passing by value for mst"
InnovativeInventor May 27, 2022
c1f73a2
Remove return reference in _minimum_spanning_tree helper
InnovativeInventor May 27, 2022
f59f7c2
Rename bipartition_graph to bipartition_graph_mst
InnovativeInventor May 27, 2022
4088b76
Create _bipartition_tree internal func
InnovativeInventor May 27, 2022
663d54e
Use numpy PyReadonlyArray to avoid one, unnecessary copy
InnovativeInventor May 27, 2022
a8f2305
Revert "Use numpy PyReadonlyArray to avoid one, unnecessary copy"
InnovativeInventor May 27, 2022
dd42700
Update pyo3 text_signature to reflect args
InnovativeInventor May 28, 2022
7c131a1
Apply suggestions from @georgois-ts
InnovativeInventor May 28, 2022
aa24d61
Update pyo3 text_signature to reflect Rust args
InnovativeInventor May 28, 2022
e7fdffe
Switch to using LinkedList for cheaper appends and remove unnecessary…
InnovativeInventor May 28, 2022
4dc3ba7
Remove LinkedList use
InnovativeInventor May 28, 2022
90df5ac
Merge remote-tracking branch 'origin/main' into feat-balanced-cut-edges
IvanIsCoding Aug 1, 2022
5fd550a
Merge remote-tracking branch 'origin/main' into feat-balanced-cut-edges
IvanIsCoding Aug 1, 2022
930069a
Move test file to rustworkx tests
IvanIsCoding Aug 1, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 32 additions & 20 deletions src/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@

use hashbrown::HashSet;
use std::cmp::Ordering;
use std::collections::LinkedList;
use std::collections::VecDeque;
use std::mem;

use super::{graph, weight_callable};

Expand Down Expand Up @@ -188,23 +190,25 @@ fn _bipartition_tree(
) -> Vec<(usize, Vec<usize>)> {
let mut pops = pops;
let spanning_tree_graph = &spanning_tree.graph;
let mut same_partition_tracker: Vec<Vec<usize>> =
vec![vec![]; spanning_tree_graph.node_bound()]; // Keeps track of all all the nodes on the same side of the partition
let mut same_partition_tracker: Vec<LinkedList<usize>> =
vec![LinkedList::new(); spanning_tree_graph.node_bound()]; // Keeps track of all all the nodes on the same side of the partition
InnovativeInventor marked this conversation as resolved.
Show resolved Hide resolved

let mut node_queue: VecDeque<NodeIndex> = VecDeque::<NodeIndex>::new();
for leaf_node in spanning_tree_graph.node_indices() {
if spanning_tree_graph.neighbors(leaf_node).count() == 1 {
node_queue.push_back(leaf_node);
}
same_partition_tracker[leaf_node.index()].push(leaf_node.index());
same_partition_tracker[leaf_node.index()].push_back(leaf_node.index());
}

// BFS search for balanced nodes
let mut balanced_nodes: Vec<(usize, Vec<usize>)> = vec![];
// BFS search for balanced nodes using LinkedList since append is O(1)
let mut balanced_nodes: Vec<(usize, LinkedList<usize>)> = vec![];
let mut seen_nodes = HashSet::with_capacity(spanning_tree_graph.node_count());
while !node_queue.is_empty() {
let node = node_queue.pop_front().unwrap();
let pop = pops[node.index()];
if seen_nodes.contains(&node.index()) {
continue;
}

// Mark as seen; push to queue if only one unseen neighbor
let unseen_neighbors: Vec<NodeIndex> = spanning_tree
Expand All @@ -214,32 +218,40 @@ fn _bipartition_tree(
.collect();

if unseen_neighbors.len() == 1 {
// This will be false if root
// At leaf, will be false at root
let pop = pops[node.index()];

// Update neighbor pop
let neighbor = unseen_neighbors[0];
pops[neighbor.index()] += pop;
let mut current_partition_tracker = same_partition_tracker[node.index()].clone();
same_partition_tracker[neighbor.index()].append(&mut current_partition_tracker);

if !node_queue.contains(&neighbor) {
node_queue.push_back(neighbor);
// Check if balanced; mark as seen
if pop >= pop_target * (1.0 - epsilon) && pop <= pop_target * (1.0 + epsilon) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Reading the docs where a balanced edge is defined and the linked paper I'd guess that pop_target should be equal to the total sum of the population divided by 2. Is there any reason why we allow users to define a different value of pop_target? I'm asking since depending of the value of pop_target this check might fail for the other part of the partition.

balanced_nodes.push((node.index(), same_partition_tracker[node.index()].clone()));
}
seen_nodes.insert(node.index());

// Update neighbor partition tracker
let mut current_partition_tracker =
mem::take(&mut same_partition_tracker[node.index()]);
same_partition_tracker[neighbor.index()].append(&mut current_partition_tracker);

// Queue neighbor
node_queue.push_back(neighbor);
} else if unseen_neighbors.is_empty() {
// root
// Is root
break;
} else {
// Not at the leaves of the unseen subgraph
// Not a leaf yet
continue;
}

// Check if balanced
if pop >= pop_target * (1.0 - epsilon) && pop <= pop_target * (1.0 + epsilon) {
balanced_nodes.push((node.index(), same_partition_tracker[node.index()].clone()));
}

seen_nodes.insert(node.index());
}

// Convert LinkedList back to vec
balanced_nodes
.iter()
.map(|(node, partition_nodes)| (*node, partition_nodes.iter().copied().collect()))
.collect()
InnovativeInventor marked this conversation as resolved.
Show resolved Hide resolved
}

/// Bipartition graph into two contiguous, population-balanced components using
Expand Down