-
Notifications
You must be signed in to change notification settings - Fork 6
Created a new code for Chu-Liu (Minimum arborescence) #59
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds an implementation of the Chu-Liu/Edmonds' algorithm for finding the minimum spanning arborescence (directed minimum spanning tree) in a directed graph.
Changes:
- Implements the Chu-Liu/Edmonds' algorithm with O(n m) complexity
- Provides a template-based structure that supports different weight types
- Includes cycle detection and contraction logic for finding the optimal arborescence
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for (int i = 0; i < n; i++) | ||
| if (id[i] < 0) | ||
| id[i] = ++tot; | ||
| n = tot + 1; |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modifying the member variable 'n' during algorithm execution (line 57) breaks the invariant that member vectors pre, num, id, vis, and inw are sized according to the original 'n' from the constructor. After this line, 'n' becomes the number of contracted nodes, but these vectors remain at their original size. While this happens to work because the algorithm only accesses indices within the new contracted node range, it makes the code fragile and could lead to bugs if the implementation is modified. Consider using a local variable for the contracted graph size instead.
| @@ -0,0 +1,79 @@ | |||
| /* | |||
| *Description:* Minimum spanning arborescence (directed MST), Edmonds/Chu-Liu algorithm | |||
| *Complexity:* $O(n m)$, returns -1 if not reachable | |||
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The description mentions returning -1 if "not reachable", but it would be clearer to specify "returns -1 if not all nodes are reachable from the root" or "returns -1 if no spanning arborescence exists rooted at the given root node". The current phrasing is somewhat ambiguous about what needs to be reachable from what.
| *Complexity:* $O(n m)$, returns -1 if not reachable | |
| *Complexity:* $O(n m)$, returns -1 if not all nodes are reachable from the root (i.e., if no spanning arborescence exists rooted at the given root node) |
| inw[root] = 0; | ||
| for (int i = 0; i < n; i++) | ||
| if (inw[i] == INF) | ||
| return -1; |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function returns -1 to indicate failure (line 37), but the return type is T (the template parameter for edge weights). If T is an unsigned type (e.g., unsigned int, unsigned long long), returning -1 will result in a large positive value instead of a negative sentinel value. Consider documenting that T must be a signed type, or use a different error handling mechanism such as std::optional or throwing an exception.
| inc.push_back(0), dec.push_back(0), take.push_back(0); | ||
| e.push_back({x, y, (int)e.size(), w}); | ||
| } | ||
| T solve(int root) { |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no validation that the root parameter is within valid bounds [0, n). If an invalid root value is passed (e.g., negative or >= n), it could lead to out-of-bounds array access at line 34 (inw[root] = 0) and throughout the function. Consider adding a check or documenting that the caller must ensure root is valid.
| T solve(int root) { | |
| T solve(int root) { | |
| if (root < 0 || root >= n) | |
| return -1; |
| chu_liu(int n) : n(n), pre(n), num(n), id(n), vis(n), inw(n) { | ||
| INF = numeric_limits<T>::max() / 2; | ||
| } | ||
| void add_edge(int x, int y, T w) { |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no validation that the vertex indices x and y are within valid bounds [0, n). If invalid vertex indices are passed to add_edge, it could lead to out-of-bounds array access when these edges are processed in the solve function (e.g., at lines 32-33 when accessing inw[ed.v], pre[ed.v], etc.). Consider adding bounds checking or documenting that the caller must ensure vertices are valid.
| void add_edge(int x, int y, T w) { | |
| void add_edge(int x, int y, T w) { | |
| // Ensure vertex indices are within valid bounds [0, n) | |
| if (x < 0 || x >= n || y < 0 || y >= n) { | |
| return; | |
| } |
No description provided.