Skip to content

Build tree optimization #280

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.

Sign up for GitHub

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

Open
vivaansinghvi07 wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Build tree optimization #280

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ee7927e
change threadhold to S - 1 to allow zero-gen stuff
vivaansinghvi07 Feb 13, 2026
225a28a
remove children generator (just using the view)
vivaansinghvi07 Feb 13, 2026
5511f09
relax requirement of sorted children arrays
vivaansinghvi07 Feb 13, 2026
176e76e
Revert "change threadhold to S - 1 to allow zero-gen stuff"
vivaansinghvi07 Feb 15, 2026
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
105 changes: 11 additions & 94 deletions hstrat/phylogenetic_inference/tree/_impl/_build_tree_searchtable_cpp_impl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,45 +415,6 @@ Records collapse_unifurcations(Records &records, const bool dropped_only) {
return new_records;
}

/**
* A makeshift iterator for children, called like this:
*
* ChildrenGenerator iter(records, node);
* while ((child = iter.next())) {
* ...
* }
*
* Returns a value of 0 when there is no more children.
*/
class ChildrenGenerator {
private:
const Records &records;
u64 prev;
const u64 node;

public:
ChildrenGenerator(
const Records &records, const u64 node
) : records(records), prev(0), node(node) {}

u64 next() {
u64 cur;
if (!this->prev) {
this->prev = this->node;
cur = this->records.search_first_child_id[this->node];
} else {
cur = this->records.search_next_sibling_id[this->prev];
}
if (this->prev == cur) {
return 0;
}
this->prev = cur;
return this->prev;
}

};


/**
* A more permissive declval.
*/
Expand Down Expand Up @@ -595,55 +556,23 @@ void attach_search_parent(Records &records, const u64 node, const u64 parent) {
records.search_ancestor_id[node] = parent;
assert(parent <= node);
assert(records.rank[parent] <= records.rank[node]);

const u64 ancestor_first_child = records.search_first_child_id[parent];
assert(ancestor_first_child != placeholder_value);
assert(records.search_first_child_id[parent] != placeholder_value);

// insert node into the list of children, choosing its position in order
// to keep the list in ascending order by rank
const i64 rank = records.rank[node];
const auto siblings = ChildrenView(records, parent);
u64 precursor_id = parent;
const auto next_sibling_it = std::ranges::find_if(
siblings,
[&records, &precursor_id, rank](const u64 sibling){
const bool res = records.rank[sibling] >= rank;
if (!res) precursor_id = sibling;
return res;
}
);

const auto next_sibling_it = siblings.begin();
const bool has_next_sibling = next_sibling_it != siblings.end();
const bool has_prev_sibling = next_sibling_it != siblings.begin();

if (has_prev_sibling) {
assert(
!has_next_sibling
|| records.search_prev_sibling_id[*next_sibling_it] == precursor_id
);
assert(precursor_id != parent);
records.search_next_sibling_id[precursor_id] = node;
records.search_prev_sibling_id[node] = precursor_id;
} else {
assert(precursor_id == parent);
records.search_first_child_id[parent] = node;
records.search_prev_sibling_id[node] = node;
}
records.search_first_child_id[parent] = node;
records.search_prev_sibling_id[node] = node;

if (has_next_sibling) {
records.search_prev_sibling_id[*next_sibling_it] = node;
records.search_next_sibling_id[node] = *next_sibling_it;
} else {
records.search_next_sibling_id[node] = node;
}

assert(std::ranges::is_sorted(
ChildrenView(records, parent),
[&records](const u64 lhs, const u64 rhs) {
return records.rank[lhs] < records.rank[rhs];
}
));

}


Expand Down Expand Up @@ -696,9 +625,7 @@ struct pairhash {
*/
void collapse_indistinguishable_nodes_large(Records &records, const u64 node) {
std::unordered_map<std::pair<i64, u64>, std::vector<u64>, pairhash> groups;
ChildrenGenerator gen(records, node);
u64 child;
while ((child = gen.next())) {
for (u64 child : ChildrenView(records, node)) {
std::vector<u64> &items = groups[
std::pair{records.rank[child], records.differentia[child]}
];
Expand All @@ -710,9 +637,7 @@ void collapse_indistinguishable_nodes_large(Records &records, const u64 node) {
const u64 loser = children[i];

std::vector<u64> loser_children;
ChildrenGenerator loser_children_gen(records, loser);
u64 loser_child;
while ((loser_child = loser_children_gen.next())) {
for (u64 loser_child : ChildrenView(records, loser)) {
loser_children.push_back(loser_child);
}
for (const u64 loser_child : loser_children) {
Expand Down Expand Up @@ -790,28 +715,20 @@ void consolidate_trie(Records &records, const i64 rank, const u64 node) {
[&records, rank](const u64 child){ return records.rank[child] <= rank; }
));

if (
children_range.begin() == children_range.end()
// chidlren are stored in ascending order by rank
|| records.rank[*children_range.begin()] == rank
) [[likely]] {
assert(std::ranges::all_of(
if (std::ranges::all_of(
children_range,
[&records, rank](const u64 child){ return records.rank[child] == rank; }
));
)) [[likely]] {
return;
}

// children are stored in ascending order by rank, so this is equivalent
// to copy_if < rank
std::vector<u64> node_stack;
const auto copy_end = std::ranges::find_if(
std::ranges::copy_if(
children_range,
[&records, rank](const u64 node){ return records.rank[node] == rank; }
);
std::ranges::copy(
std::ranges::subrange(children_range.begin(), copy_end),
std::back_inserter(node_stack)
std::back_inserter(node_stack),
[&records, rank](const u64 node){ return records.rank[node] < rank; }
);

assert(!node_stack.empty());
Expand Down
Loading