Skip to content

Conversation

@andreacassioli
Copy link

@andreacassioli andreacassioli commented Dec 5, 2025

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:

  1. read/write a fixed integer type to allow different graph types (with potentially different size types) to be used
  2. read/write integers that depend on the graph type, loosing potentially some fleibility

I believe the former is the original design. So in this PR

  • make sure all read/write serializations are down using unsigned int
  • convert the relevant values to the type deduced from the graph traits
  • small cleanup modernizing a bit the code

@andreacassioli
Copy link
Author

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.

@andreacassioli andreacassioli marked this pull request as ready for review December 30, 2025 15:44
@andreacassioli
Copy link
Author

@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?

@jeremy-murphy
Copy link
Contributor

@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;
Copy link
Contributor

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?

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.

2 participants