Skip to content

Conversation

@tlively
Copy link
Member

@tlively tlively commented Aug 29, 2024

Reuse the code implementing Kahn's topological sort algorithm with a new
configuration that uses a min-heap to always choose the best available
element.

Also add wrapper utilities that can find topological sorts of graphs
with arbitrary element types, not just indices.

Reuse the code implementing Kahn's topological sort algorithm with a new
configuration that uses a min-heap to always choose the best available
element.

Also add wrapper utilities that can find topological sorts of graphs
with arbitrary element types, not just indices.
@tlively tlively requested a review from kripken August 29, 2024 03:36
// the indices of its children, which will appear after it in the order.
TopologicalOrders(const std::vector<std::vector<size_t>>& graph);
TopologicalOrders(const std::vector<std::vector<size_t>>& graph)
: TopologicalOrders(graph, false) {}
Copy link
Member

Choose a reason for hiding this comment

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

Maybe an enum for this?

const std::vector<size_t>& operator*() const {
return TopologicalOrders::operator*();
}
};
Copy link
Member

Choose a reason for hiding this comment

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

This constructor is identical to the parent, and this operator doesn't seem to modify anything either - I am sure I'm missing something?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, this is providing a small subset of the functionality of the superclass, packaged under a different name that is more appropriate for the common case of needing just one topological sort.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, thanks... I was missing the word "private" on line 108. Makes sense now.

Graph graph(3);
graph[2].push_back(1);
std::vector<size_t> expected{0, 2, 1};
EXPECT_EQ(*MinTopologicalSort(graph), expected);
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add comments to these, e.g. for this one IIUC the point is to show that 2 is before 1, and the unconstrained 0 is smaller so it appears before them both.

@tlively tlively merged commit b63aead into main Aug 29, 2024
@tlively tlively deleted the heap-toposort branch August 29, 2024 22:07
@gkdn gkdn mentioned this pull request Aug 31, 2024
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.

3 participants