-
Notifications
You must be signed in to change notification settings - Fork 229
Fix adjacency list serialise types #443
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
base: develop
Are you sure you want to change the base?
Fix adjacency list serialise types #443
Conversation
|
I think this might break compatibility because the size of the integers written and read will depends on the graphs used to read and write. So I believe there is a bit more to do. |
|
@jeremy-murphy there is a failure in the CI that I do not understand, it does seem to be related to my changes. What do you think? |
Yeah, that's very odd. Looks like a problem either in Boost.Math or in the system libraries themselves. I would just ignore it for now and hope it goes away by itself. |
| typedef typename graph_traits< Graph >::vertex_descriptor Vertex; | ||
| using Graph = adjacency_list< OEL, VL, D, VP, EP, GP, EL >; | ||
| using Vertex = typename graph_traits< Graph >::vertex_descriptor; | ||
| using SerializedInteger = unsigned int; |
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.
Isn't this going to cause a problem for users with, however unlikely, graphs where vertices_size_type is std::size_t and actually have billions or trillions or vertices?
Also, it would waste space for users that have billions of tiny graphs where they have customized vertices_size_type to be unsigned char, etc?
Try to address this in a more general way.
According to the offical docs the type representing the number of vertices and edges in a graph can be deduced by the graph traits.
IMHO there are two approaches:
I believe the former is the original design. So in this PR
unsigned int