Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Defining pow functions for adjacency and transition matrices #463
base: master
Are you sure you want to change the base?
Defining pow functions for adjacency and transition matrices #463
Changes from 2 commits
ad3bef6
74a4cee
fd81892
d28c527
a245851
af23687
e284c74
1d7dcf9
02453a9
03346c1
14bbbdc
7e37bb1
0f1de02
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I would use a standard algorithm for this, like
std::transform
,std::fill
orstd::generate
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.
Why two specific methods? I think that one method overloaded two times would be better.
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.
Okay! I'll just have the user pass an enum value to distinguish. Or, would you prefer they pass the actual matrices themselves to the function? I just want to make sure I'm being consistent with the rest of the repo's style.
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.
virtual const PowAdjResult matrixPow(unsigned int k, AdjacencyMatrix adj) const
with usage like:
graph.matrixPow(2, graph.getAdjMatrix());
This feels wrong to me... should I make a function w/ is not a member of the CXXGraph class? That also feels wrong...
What kind of overloading are you thinking?
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.
Hi, sorry for the delay, I'm very busy these weeks. For me the best thing would be to implement them as free functions, not as Graph methods. But we can hear what @ZigRazor thinks.
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.
Sounds good! No worries!
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.
Hey @ZigRazor
Just following up here - would like to get this wrapped up soon! Thoughts on implementing this as a free function?
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.
Sorry for the delay.
Yes, the best choice for me is to implement them as a free functions.
Thank you
Check warning on line 110 in include/CXXGraph/Graph/Algorithm/Pow_impl.hpp
include/CXXGraph/Graph/Algorithm/Pow_impl.hpp#L110