-
Notifications
You must be signed in to change notification settings - Fork 34
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
Floyd-Warshall algorithm #98
Conversation
Corrected trailling slashes
- relative railway example - corrected png files
- Added test cases and docs
Added one test
Corrected doc file
- algorithm - test - documentation
Codecov ReportAttention:
... and 2 files with indirect coverage changes 📢 Thoughts on this report? Let us know!. |
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.
Looks very good already!
I will take a closer look tomorrow but wanted to provide some early feedback :)
template <typename V, typename E, graph_type T, typename WEIGHT_T> | ||
std::vector<std::vector<WEIGHT_T>> floyd_warshall_shortest_paths( | ||
const graph<V, E, T>& graph) { | ||
WEIGHT_T ZERO{}; | ||
std::size_t n = graph.vertex_count(); | ||
auto INF = std::numeric_limits<WEIGHT_T>::max(); | ||
auto NO_PATH = std::numeric_limits<WEIGHT_T>::min(); |
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.
Regarding using std::numeric_limits<WEIGHT_T>::min()
for vertex pairs between which we cannot find a path (i.e. vertices in negative weight cycles), consider the case where the WEIGHT_T
is an unsigned
type.
In that case, the caller cannot distinguish between a NO_PATH
and a true distance of 0
. However, I do not see an elegant way of handling this. What do you think about removing the negative cycle detection, as it is also documented on the interface that the algorithm does not work in this case.
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.
Oh, I didn't see this one. My bad. I'll fix it right away. Agreed, we could move it to a different PR as Floyd-Warshall negative cycle detection so results will be less confusing. And in this case, NO_PATH will no longer be needed.
I look forward to your next comments about PR. :)
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.
Thanks for working on this, LGTM 🚀
Added Floyd-Warshall alogorithm, tests and docs.