From 8c56cc480b94816e714d32bbf55150858b910333 Mon Sep 17 00:00:00 2001 From: Phil Ratzloff Date: Mon, 8 Jun 2026 10:20:47 -0400 Subject: [PATCH 1/9] Align views spec with current API and traversal semantics --- D3129_Views/tex/views.tex | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/D3129_Views/tex/views.tex b/D3129_Views/tex/views.tex index 72f84fe..b33ce3c 100644 --- a/D3129_Views/tex/views.tex +++ b/D3129_Views/tex/views.tex @@ -65,7 +65,7 @@ \section{Data Structs (Return Types)}\label{sec:data-structs} \end{lstlisting} \subsection{\tcode{struct vertex_data}}\label{vertex-view}\mbox{} \\ -\tcode{vertex_data} is used to return vertex information. It is used by \tcode{vertexlist(g)}, \tcode{vertices_breadth_first_search(g,u)}, +\tcode{vertex_data} is used to return vertex information. It is used by \tcode{vertexlist(g)}, \tcode{vertices_bfs(g,u)}, \tcode{vertices_dfs(g,u)} and others. The \tcode{id} member is the vertex id, the \tcode{vertex} member is the vertex descriptor, and \tcode{value} is the result of the value function, if provided. @@ -100,7 +100,7 @@ \subsection{\tcode{struct vertex_data}}\label{vertex-view}\mbox{} \\ \subsection{\tcode{struct edge_data}}\label{edge-view}\mbox{} \\ \tcode{edge_data} is used to return edge information. It is used by -\tcode{incidence(g,u), edgelist(g), edges_breadth_first_search(g,u), edges_dfs(g,u)} and others. +\tcode{incidence(g,u), edgelist(g), edges_bfs(g,u), edges_dfs(g,u)} and others. \tcode{source_id} and \tcode{target_id} are vertex ids of type \tcode{vertex_id_t}. When \tcode{Sourced=true}, the \tcode{source_id} member is included. The \tcode{target_id} member always exists. @@ -163,9 +163,11 @@ \subsection{\tcode{struct neighbor_data}}\label{neighbor-vi \hline \tcode{neighbor_data} & \tcode{source_id} & \tcode{target_id} & \tcode{target} & \tcode{value} \\ \tcode{neighbor_data} & \tcode{source_id} & \tcode{target_id} & \tcode{target} & \\ + \tcode{neighbor_data} & \tcode{source_id} & \tcode{target_id} & & \tcode{value} \\ \tcode{neighbor_data} & \tcode{source_id} & \tcode{target_id} & & \\ \tcode{neighbor_data} & & \tcode{target_id} & \tcode{target} & \tcode{value} \\ \tcode{neighbor_data} & & \tcode{target_id} & \tcode{target} & \\ + \tcode{neighbor_data} & & \tcode{target_id} & & \tcode{value} \\ \tcode{neighbor_data} & & \tcode{target_id} & & \\ \hline \end{tabular}} @@ -387,8 +389,8 @@ \subsection{Common Types and Functions for ``Search'' } \begin{lstlisting} cancel_search cancel() const noexcept; // get current cancel state void cancel(cancel_search c) noexcept; // set cancel state -std::size_t depth() const noexcept; // distance from seed to current vertex -std::size_t num_visited() const noexcept; // total vertices/edges visited so far +std::size_t depth() const noexcept; // depth of currently yielded element (seed is 0) +std::size_t num_visited() const noexcept; // elements yielded so far, including the current element \end{lstlisting} The \tcode{search_view} concept captures these requirements: @@ -403,12 +405,15 @@ \subsection{Common Types and Functions for ``Search'' } Topological sort views support \tcode{cancel()} and \tcode{num_visited()} but not \tcode{depth()} (flat ordering has no tree structure), so they do not satisfy \tcode{search_view}. -For DFS, \tcode{dfs.num_visited()} is typically close to \tcode{dfs.depth()} and is simple to calculate. Breadth-first search requires extra bookkeeping to evaluate \tcode{bfs.depth()}, which returns a different value than \tcode{bfs.num_visited()}. +For views that satisfy \tcode{search_view} (DFS and BFS), \tcode{depth()} denotes the depth of the currently yielded element measured from the seed. The seed has depth 0, and each traversed edge increases depth by 1. + +\tcode{num_visited()} returns the number of elements yielded so far, \emph{including the element currently held by the loop variable}. On the first iteration it equals 1 (the seed for vertex views, the first tree edge for edge views). This makes threshold checks inside the loop body intuitive: when the loop body executes, \tcode{num_visited()} reflects the current position rather than the previous one. The following example shows how the member functions could be used, using \tcode{dfs} for one of the depth-first search views. The same members are available on all search views that satisfy \tcode{search_view}. \begin{lstlisting} auto&& g = ...; // graph auto&& dfs = vertices_dfs(g,0); // start with seed vertex\_id=0 +// On first iteration: dfs.depth()==0, dfs.num\_visited()==1 for(auto&& [uid,u] : dfs) { // No need to search deeper? if(dfs.depth() > 3) { @@ -533,7 +538,7 @@ \subsection{Topological Sort Views} \section{Range Adaptors (Pipe Syntax)} -All view factory functions described in this paper are also available as range adaptor closure objects, enabling pipe syntax. The adaptor objects live in \tcode{namespace graph::views} and forward to the corresponding factory function. Table \ref{tab:adaptors_graph} shows the graph view adaptors and Table \ref{tab:adaptors_search} shows the search view adaptors. +All range-returning, non-\tcode{_safe} view factories listed in Tables \ref{tab:adaptors_graph} and \ref{tab:adaptors_search} are also available as range adaptor closure objects, enabling pipe syntax. The adaptor objects live in \tcode{namespace graph::views} and forward to the corresponding factory function. The \tcode{_safe} topological-sort factories return \tcode{std::expected} and are not adaptor closures. \tcode{transpose} is a graph adaptor (not a range adaptor) and remains documented separately. \begin{table}[h!] \begin{center} From 18799a598daf042bf80452bebafd1901b1fc2838 Mon Sep 17 00:00:00 2001 From: Phil Ratzloff Date: Mon, 8 Jun 2026 22:31:33 -0400 Subject: [PATCH 2/9] D3130: split edge concept into basic_edge + edge; add GCI review notes Update the Container Interface paper to match the implementation: document the shared basic_edge concept (source_id + target_id) and the adj_list edge refinement that adds source/target descriptors, with basic_sourced_edgelist resting on basic_edge. Add agents/D3130_review.md capturing the full code-vs-paper review and open questions. --- .../src/concepts_edges.hpp | 11 +- .../src/edgelist_concepts.hpp | 5 +- .../tex/container_interface.tex | 23 ++- agents/D3130_review.md | 192 ++++++++++++++++++ 4 files changed, 220 insertions(+), 11 deletions(-) create mode 100644 agents/D3130_review.md diff --git a/D3130_Container_Interface/src/concepts_edges.hpp b/D3130_Container_Interface/src/concepts_edges.hpp index 6718c69..1b7ebbb 100644 --- a/D3130_Container_Interface/src/concepts_edges.hpp +++ b/D3130_Container_Interface/src/concepts_edges.hpp @@ -1,9 +1,16 @@ // For exposition only +// Shared edge floor (namespace std::graph): ids only. template -concept edge = requires(G& g, const E& e) { +concept basic_edge = requires(G& g, const E& e) { source_id(g, e); - source(g, e); target_id(g, e); +}; + +// Adjacency-list refinement (namespace std::graph::adj\_list): +// adds the source/target vertex descriptors. +template +concept edge = basic_edge && requires(G& g, const E& e) { + source(g, e); target(g, e); }; diff --git a/D3130_Container_Interface/src/edgelist_concepts.hpp b/D3130_Container_Interface/src/edgelist_concepts.hpp index 1cfb8f5..30eaac8 100644 --- a/D3130_Container_Interface/src/edgelist_concepts.hpp +++ b/D3130_Container_Interface/src/edgelist_concepts.hpp @@ -5,10 +5,7 @@ template concept basic_sourced_edgelist = ranges::input_range && !ranges::range> && - requires(EL& el, ranges::range_value_t uv) { - { source_id(el, uv) }; - { target_id(el, uv) } -> convertible_to; - }; + basic_edge>; // shared edge floor: source\_id + target\_id template concept basic_sourced_index_edgelist = diff --git a/D3130_Container_Interface/tex/container_interface.tex b/D3130_Container_Interface/tex/container_interface.tex index 85c4887..2927e3d 100644 --- a/D3130_Container_Interface/tex/container_interface.tex +++ b/D3130_Container_Interface/tex/container_interface.tex @@ -57,8 +57,18 @@ \subsection{Concepts} until we have consensus of whether they belong in the standard or not.} \subsubsection{Edge Concepts} -The \tcode{edge} concept unifies all edge types in a single concept: an edge must provide \tcode{source_id}, -\tcode{source}, \tcode{target_id}, and \tcode{target} access given the graph. +Two layered edge concepts are defined. \tcode{basic_edge} is the shared floor: an edge must +provide \tcode{source_id(g,e)} and \tcode{target_id(g,e)}. It lives in \tcode{std::graph} and depends only +on the shared id CPOs, so it ties an edge to both the adjacency list and the edge list (it is the concept +required by \tcode{basic_sourced_edgelist}, below). + +The adjacency-list \tcode{edge} concept refines \tcode{basic_edge} by additionally requiring the +\tcode{source(g,e)} and \tcode{target(g,e)} vertex descriptors. Within an \tcode{adjacency_list} these are +always available: \tcode{out_edges(g,u)} and \tcode{in_edges(g,u)} always yield edge descriptors, and +\tcode{adjacency_list} implies a \tcode{vertex_range} (hence \tcode{find_vertex}), so the default +\tcode{source}/\tcode{target} resolve to \tcode{*find_vertex(g, source_id/target_id(g,e))}. The descriptor +requirement is therefore free for conforming graphs while keeping edge-list elements --- which have no +vertex container --- out of the adjacency-list \tcode{edge} concept; those satisfy only \tcode{basic_edge}. {\small \lstinputlisting{D3130_Container_Interface/src/concepts_edges.hpp} } @@ -637,7 +647,8 @@ \subsection{Namespace} neighbour traversal; an edge list is a flat range of \tcode{(source, target [, value])} tuples suited to edge-centric algorithms such as Kruskal's MST or bulk construction. CPOs that operate directly on edge data --- \tcode{source\_id}, \tcode{target\_id} and \tcode{edge\_value} --- are shared and -live in \tcode{std::graph}. +live in \tcode{std::graph}, as is the \tcode{basic\_edge} concept built on them, which both the +adjacency-list \tcode{edge} concept and the edge-list \tcode{basic\_sourced\_edgelist} concept refine. \subsection{Concepts} @@ -645,8 +656,10 @@ \subsection{Concepts} matching the adjacency list CPO convention. All edgelist CPOs require both the container and the element to allow for container-specific dispatch. -\tcode{basic_sourced_edgelist} is the base concept: an input range whose element type is not itself a range, supporting -\tcode{source_id(el,uv)} and \tcode{target_id(el,uv)} with compatible types. \tcode{basic_sourced_index_edgelist} additionally +\tcode{basic_sourced_edgelist} is the base concept: an input range whose element type is not itself a range, whose element +satisfies the shared \tcode{basic_edge} concept (\tcode{source_id(el,uv)} and \tcode{target_id(el,uv)}). +This is the same \tcode{basic_edge} floor required by the adjacency-list \tcode{edge} concept, tying the two abstract data types +together at the edge level. \tcode{basic_sourced_index_edgelist} additionally requires both ids to be integral. \tcode{has_edge_value} refines \tcode{basic_sourced_edgelist} to also provide \tcode{edge_value(el,uv)}. {\small \lstinputlisting{D3130_Container_Interface/src/edgelist_concepts.hpp} diff --git a/agents/D3130_review.md b/agents/D3130_review.md new file mode 100644 index 0000000..325bbe6 --- /dev/null +++ b/agents/D3130_review.md @@ -0,0 +1,192 @@ +# D3130 Container Interface — Review Comments + +Review of `D3130_Container_Interface/tex/container_interface.tex` against the +reference implementation in `/home/phil/dev_graph/graph-v3/include/graph/`. + +Scope reviewed: full GCI paper text — adjacency-list concepts/traits/types/functions, +descriptors, vertex property map, value-function concepts, partitions, and the +edgelist interface. Focus is clarity, consistency (paper vs. code), and completeness. +Topics owned by sibling papers (algorithms, views, containers) are noted but not +re-reviewed here. + +Legend: **[HIGH]** = code/paper genuinely disagree on the interface; **[MED]** = +documented symbol/behavior absent or different in the reference impl; **[LOW]** = +wording/snippet polish. + +--- + +## 1. Code-vs-paper inconsistencies + +### 1.1 [HIGH] `edge` concept — RESOLVED via `basic_edge`/`edge` split +- Original mismatch: paper required `source_id`/`source`/`target_id`/`target`; + code required only `source_id`/`target_id`. +- Resolution (implemented in code + paper): introduced a shared + `graph::basic_edge` concept (ids only) and redefined the adjacency-list + `adj_list::edge` to refine `basic_edge` plus `source(g,e)`/`target(g,e)`. + `edge_list::basic_sourced_edgelist` now refines `basic_edge`, tying both ADTs + to one shared edge floor. Edge-list elements (raw tuples/pairs/`edge_data`) + satisfy `basic_edge` but not `adj_list::edge` (no vertex container to resolve + descriptors). Verified: adj_list / container / views / edge_list suites pass; + edge_list interop test updated to assert the new contract. + +### 1.2 [HIGH] `edge_descriptor::edge_storage_type` differs from the implementation +- Paper: `src/descriptor.hpp` shows + `edge_storage_type = conditional_t, size_t, EdgeIter>`, + and the surrounding prose ("Index-based: one integer") implies an index is + stored for random-access edge containers. +- Code: `adj_list/edge_descriptor.hpp` — `using edge_storage_type = EdgeIter;` + (always the iterator; no index specialization). The `vertex_descriptor` + *does* use the `size_t`/iterator conditional, but the `edge_descriptor` does + not. +- Action: fix the `edge_descriptor` snippet to store `EdgeIter`, and adjust the + copy-size discussion so the index claim applies to `vertex_descriptor` only. + +### 1.3 [MED] Edgelist `vertex_id_t`: paper says `target_id`, code uses `source_id` +- Paper: Table "Edgelist Interface Type Aliases" defines + `vertex_id_t = remove_cvref_t`, and the + Namespace section says the edgelist `vertex_id_t` "is derived ... from + `target_id(el,uv)`." +- Code: `edge_list/edge_list.hpp` derives both `vertex_id_t` and + `raw_vertex_id_t` from **`source_id(el,uv)`**. +- The two ids are constrained to be the same type, so this is harmless at + runtime, but the documentation should match the implementation (or vice + versa). Pick one consistently. + +### 1.4 [MED] `vertex_id_store_t` does not exist +- Paper: Table "Graph Container Interface Type Aliases" lists + `vertex_id_store_t` and labels it "normative". +- Code: no such alias. The closest is `raw_vertex_id_t` (the paper's + "*raw-vertex-id-type*", exposition only). +- Action: remove `vertex_id_store_t` or replace with the actual alias name + and definition. + +### 1.5 [MED] Adjacency-matrix trait family is undocumented-as-future and partly absent +- Paper: traits table lists `define_adjacency_matrix` (flagged "Not yet in + reference implementation"), plus `is_adjacency_matrix`, `is_adjacency_matrix_v`, + and the `adjacency_matrix` concept (no flag). The `contains_out_edge` default + in Table "Edge Functions" describes a constant-time path guarded by + `is_adjacency_matrix_v`. +- Code: none of `define_adjacency_matrix`, `is_adjacency_matrix`, + `is_adjacency_matrix_v`, or `adjacency_matrix` exist. The actual + `contains_edge`/`contains_out_edge` default always does a linear `find_if` — + there is no adjacency-matrix O(1) shortcut. +- Action: either (a) mark the whole family + the `contains_out_edge` constant + branch as "not yet in reference implementation", or (b) drop them. See Open + Question Q1. + +### 1.6 [MED] Partition-filtered `out_edges(g,*,pid)` not implemented +- Paper: the partitions section states `out_edges(g,uid,pid)` and + `out_edges(g,u,pid)` "filter the edges where the target is in the partition + `pid`." +- Code: no `out_edges` overload takes a `pid`. Only `vertices(g,pid)`, + `num_vertices(g,pid)`, `num_partitions(g)`, and `partition_id(g,u)` exist. +- Action: mark as future, or remove. See Open Question Q2. + +### 1.7 [MED] Edgelist functions `num_edges/has_edges/contains_edge` are TODO in code +- Paper: Table "Edgelist Interface Functions" lists `num_edges(el)`, + `has_edges(el)`, and `contains_edge(el,uid,vid)` with default implementations. +- Code: `edge_list/edge_list.hpp` header marks these "(todo)"; only + `source_id`, `target_id`, `edge_value` CPOs are implemented in `edge_cpo.hpp`. +- Action: confirm intent — keep documented as the target interface, or add a + "not yet implemented" note. See Open Question Q3. + +### 1.8 [LOW] `vertex_value(g,uid)` convenience overload missing in code +- Paper: vertex-functions table lists `vertex_value(g,uid)` with default + `vertex_value(g,*find_vertex(g,uid))`. +- Code: `_vertex_value` CPO provides only `operator()(g, u)` (descriptor form); + no `uid` overload. +- Action: either add the overload to the impl or footnote it as not-yet-provided. + +### 1.9 [LOW] `mapped_vertex_range` snippet omits the `hashable_vertex_id` requirement +- Paper: `src/concepts_vertex_range.hpp` shows `mapped_vertex_range` as + `!index_vertex_range && requires { vertices(g) -> forward_range; find_vertex(g,uid); }`. +- Code: also requires `hashable_vertex_id` (vertex id must be usable as an + `unordered_map` key). The `hashable_vertex_id` concept is not mentioned in the + paper at all. +- Action: add `hashable_vertex_id` to the snippet and a one-line description, + or note why it is intentionally elided. + +### 1.10 [LOW] `vertex` snippet omits the `is_vertex_descriptor_v` guard +- Paper: `src/concepts_vertex_range.hpp` `vertex` concept requires + `vertex_id(g,u)` and `find_vertex(g,uid)`. +- Code: additionally requires `is_vertex_descriptor_v>`. +- Action: add the guard to the snippet (minor, but it is load-bearing for + disambiguating descriptors from raw values). + +--- + +## 2. Clarity / completeness observations + +### 2.1 [LOW] Partition default-implementation semantics understated +- Paper defaults: `vertices(g,pid)` → `vertices(g)`; `num_vertices(g,pid)` → + `size(vertices(g))`. +- Code defaults: return the full range/count **only when `pid==0`**, otherwise + an empty range / `0`. This matches the "one partition holds everything" model + but the table omits the `pid==0` condition, which could mislead implementers. +- Action: state the defaults as "for `pid==0`, returns all vertices; otherwise + empty / 0". + +### 2.2 [LOW] Undocumented `num_edges(g,u)` overload +- Code has a `num_edges(g, u)` (per-vertex) CPO in addition to `num_edges(g)`. + The paper documents only `num_edges(g)` (and `out_degree`). If `num_edges(g,u)` + is intended as public API, document it or alias it to `out_degree`; if it is + internal, no change needed. + +### 2.3 Concept count wording +- Paper: "Six concepts are defined" for the adjacency list — accurate for the + documented set. Code additionally defines `ordered_vertex_edges` (used by + triangle-count in the algorithms paper) and `hashable_vertex_id` (used by + `mapped_vertex_range`). `ordered_vertex_edges` is reasonably left to the + algorithms paper; `hashable_vertex_id` is GCI-local and probably belongs here + (see 1.9). + +--- + +## 3. Confirmed consistent (spot-checked, no action) +- Adjacency-list concepts: `adjacency_list`, `index_adjacency_list`, + `bidirectional_adjacency_list`, `index_bidirectional_adjacency_list`, + `mapped_adjacency_list`, `mapped_bidirectional_adjacency_list` — present and + matching. +- Query traits: `has_degree`, `has_find_vertex`, `has_find_vertex_edge`, + `has_contains_edge`, `has_in_degree`, `has_find_in_edge`, `has_contains_in_edge`, + `has_basic_queries`, `has_full_queries` — present, definitions match. +- `find_out_edge` primary with `find_vertex_edge` alias; `contains_out_edge` + primary with `contains_edge` alias — matches paper; both re-exported into + `graph::`. +- `vertex_property_map` and the four helpers + (`make_vertex_property_map` eager+lazy, `vertex_property_map_contains`, + `vertex_property_map_get`) — match exactly. +- `vertex_data` / `edge_data` / `neighbor_data` specialization tables and the + `copyable_vertex_t` / `copyable_edge_t` / `copyable_neighbor_t` aliases — + match `graph_data.hpp`. +- `descriptor_traits.hpp` (`is_vertex_descriptor`, `is_edge_descriptor`, + `is_descriptor` + `_v` forms) — match. +- `vertex_descriptor` storage conditional (`size_t` vs iterator), `value()`, + `vertex_id()`, `underlying_value()`, `inner_value()`, `operator++` — match. +- `out_edge_tag` / `in_edge_tag`, `is_in_edge` / `is_out_edge` on + `edge_descriptor` — match. +- Partition CPOs `num_partitions(g)`, `partition_id(g,u)`, `vertices(g,pid)`, + `num_vertices(g,pid)` — present. +- `graph_error : runtime_error` — matches. +- Edgelist concepts `basic_sourced_edgelist`, `basic_sourced_index_edgelist`, + `has_edge_value`, and the `is_directed`/`is_directed_v` trait — match. + +--- + +## 4. Open questions (scope / intent — need author input) + +- **Q1 (adjacency matrix):** Should the adjacency-matrix trait family + (`define_adjacency_matrix`, `is_adjacency_matrix`, `is_adjacency_matrix_v`, + `adjacency_matrix`) and the constant-time `contains_out_edge` branch stay in + this paper as *future design*, or be removed until implemented? (See 1.5.) +- **Q2 (partition edge filtering):** Keep `out_edges(g,uid,pid)` / + `out_edges(g,u,pid)` as documented future API, or remove? (See 1.6.) +- **Q3 (edgelist functions):** Are `num_edges(el)` / `has_edges(el)` / + `contains_edge(el,uid,vid)` part of the proposed interface (document as-is) or + should they be flagged "not yet implemented"? (See 1.7.) +- **Q4 (edgelist `vertex_id`):** Should the edgelist `vertex_id_t` be derived + from `source_id` (as the code does) or `target_id` (as the paper says)? (See + 1.3.) +- **Q5 (`hashable_vertex_id`):** Does the `hashable_vertex_id` concept belong in + this paper (it is GCI-local and underpins `mapped_vertex_range`), or is it + intentionally hidden? (See 1.9.) From c6141c65054156138e348fdb2eecc8ed6cbb4e68 Mon Sep 17 00:00:00 2001 From: Phil Ratzloff Date: Mon, 8 Jun 2026 22:35:32 -0400 Subject: [PATCH 3/9] D3130: fix edge_descriptor storage type to match implementation edge_storage_type is always EdgeIter (edges are backed by a physical container, so there is no index-only edge path). Update the descriptor.hpp snippet, the descriptor-storage prose, and scope the size claims so 'one integer' applies to the index-based vertex_descriptor while an edge_descriptor is an iterator plus the source vertex_descriptor. Resolves review item 1.2. --- D3130_Container_Interface/src/descriptor.hpp | 6 ++--- .../tex/container_interface.tex | 8 ++++--- agents/D3130_review.md | 23 ++++++++++--------- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/D3130_Container_Interface/src/descriptor.hpp b/D3130_Container_Interface/src/descriptor.hpp index 87060a6..d130d59 100644 --- a/D3130_Container_Interface/src/descriptor.hpp +++ b/D3130_Container_Interface/src/descriptor.hpp @@ -52,9 +52,9 @@ class edge_descriptor { static constexpr bool is_in_edge = is_same_v; static constexpr bool is_out_edge = is_same_v; - // index for random-access, iterator for forward (for exposition only) - using edge_storage_type = - conditional_t, size_t, EdgeIter>; + // Edges are always backed by a physical container, so the edge is + // stored as its iterator directly (no index-only path). (for exposition only) + using edge_storage_type = EdgeIter; constexpr edge_descriptor() = default; constexpr edge_descriptor(edge_storage_type edge_val, diff --git a/D3130_Container_Interface/tex/container_interface.tex b/D3130_Container_Interface/tex/container_interface.tex index 2927e3d..a93ded3 100644 --- a/D3130_Container_Interface/tex/container_interface.tex +++ b/D3130_Container_Interface/tex/container_interface.tex @@ -537,7 +537,7 @@ \subsection{Vertex and Edge Descriptor Views} Overload reduction & Separate overloads for index vs.\ iterator & A single overload accepts the descriptor \\ Invalidation & Invalidated by container mutation & Can be re-resolved via \tcode{find_vertex} \\ Copyability & Varies (e.g.\ \tcode{forward_list::iterator}) & Always trivially copyable or cheap \\ - Size & May be pointer-sized or larger & Index-based: one integer \\ + Size & May be pointer-sized or larger & Index-based vertex: one integer; edge: an iterator plus the source vertex descriptor \\ \hline \hline \end{tabular}} @@ -566,7 +566,9 @@ \subsection{Vertex and Edge Descriptor Views} \tcode{edge_t} types, respectively. \tcode{vertex_descriptor} stores either a \tcode{size_t} index (for random-access containers) or an iterator (for associative containers) as an exposition-only private member. -\tcode{edge_descriptor} additionally stores the +\tcode{edge_descriptor} always stores the edge +as its iterator (\tcode{EdgeIter}) --- unlike a vertex, an edge is always backed by a +physical container element, so there is no index-only edge path. It additionally stores the source \tcode{vertex_descriptor} and an \tcode{EdgeDirection} tag (\tcode{out_edge_tag} or \tcode{in_edge_tag}) that controls source/target semantics, allowing bidirectional graphs to use the same edge type for both outgoing and incoming edges. @@ -580,7 +582,7 @@ \subsection{Vertex and Edge Descriptor Views} Because descriptors are small and trivially copyable, CPO signatures accept them by value. A \tcode{vertex_descriptor} is at most pointer-sized (one index or iterator). -An \tcode{edge_descriptor} is larger because it also carries the source +An \tcode{edge_descriptor} is larger because it carries an edge iterator plus the source \tcode{vertex_descriptor}, but is still cheap to copy. The \tcode{const\&} form appears only in \tcode{requires} clauses of concepts where value copies are not permitted. {\small diff --git a/agents/D3130_review.md b/agents/D3130_review.md index 325bbe6..dd330ac 100644 --- a/agents/D3130_review.md +++ b/agents/D3130_review.md @@ -29,17 +29,18 @@ wording/snippet polish. descriptors). Verified: adj_list / container / views / edge_list suites pass; edge_list interop test updated to assert the new contract. -### 1.2 [HIGH] `edge_descriptor::edge_storage_type` differs from the implementation -- Paper: `src/descriptor.hpp` shows - `edge_storage_type = conditional_t, size_t, EdgeIter>`, - and the surrounding prose ("Index-based: one integer") implies an index is - stored for random-access edge containers. -- Code: `adj_list/edge_descriptor.hpp` — `using edge_storage_type = EdgeIter;` - (always the iterator; no index specialization). The `vertex_descriptor` - *does* use the `size_t`/iterator conditional, but the `edge_descriptor` does - not. -- Action: fix the `edge_descriptor` snippet to store `EdgeIter`, and adjust the - copy-size discussion so the index claim applies to `vertex_descriptor` only. +### 1.2 [HIGH] `edge_descriptor::edge_storage_type` — RESOLVED (paper now matches code) +- Original mismatch: paper showed + `edge_storage_type = conditional_t, size_t, EdgeIter>` + with prose implying an index for random-access edges; code always uses + `using edge_storage_type = EdgeIter;` (no index-only edge path, since edges + are always backed by a physical container element). +- Resolution (paper updated): `src/descriptor.hpp` snippet now shows + `edge_storage_type = EdgeIter`; the descriptor-storage prose states the edge + is always stored as its iterator; the comparison-table size row and the + copy-size paragraph now scope the "one integer" claim to the index-based + `vertex_descriptor` and describe an `edge_descriptor` as an iterator plus the + source `vertex_descriptor`. PDF rebuilds cleanly. ### 1.3 [MED] Edgelist `vertex_id_t`: paper says `target_id`, code uses `source_id` - Paper: Table "Edgelist Interface Type Aliases" defines From 5b07fda3a7ea994cb78eb9838baac66373a2d6e0 Mon Sep 17 00:00:00 2001 From: Phil Ratzloff Date: Mon, 8 Jun 2026 22:37:40 -0400 Subject: [PATCH 4/9] D3130: align edgelist vertex_id_t to source_id (matches implementation) The edgelist vertex_id_t and raw-vertex-id-type are derived from source_id(el,uv), matching edge_list.hpp. Update the type-alias table and the namespace prose (previously referenced target_id). Resolves review item 1.3 / Q4. --- .../tex/container_interface.tex | 6 ++--- agents/D3130_review.md | 25 +++++++++---------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/D3130_Container_Interface/tex/container_interface.tex b/D3130_Container_Interface/tex/container_interface.tex index a93ded3..3ff8d99 100644 --- a/D3130_Container_Interface/tex/container_interface.tex +++ b/D3130_Container_Interface/tex/container_interface.tex @@ -641,7 +641,7 @@ \subsection{Namespace} Edge list concepts, type aliases and CPOs are defined in the \tcode{std::graph::edge\_list} namespace. Unlike the adjacency list, these symbols are \emph{not} re-exported into \tcode{std::graph} because certain names have distinct meanings in each context --- most notably \tcode{vertex\_id\_t} is derived -from \tcode{vertex\_id(g,u)} for an adjacency list but from \tcode{target\_id(el,uv)} for an edge list. +from \tcode{vertex\_id(g,u)} for an adjacency list but from \tcode{source\_id(el,uv)} for an edge list. Bringing both into the root namespace would create an irresolvable ambiguity. \tcode{graph::adj\_list} and \tcode{graph::edge\_list} are \emph{peer} namespaces: neither is a @@ -709,8 +709,8 @@ \subsection{Types} \tcode{edge_t} & \tcode{range_value_t>} & \\ \tcode{edge_value_t} & \tcode{decltype(edge_value(el,uv))} & optional \\ \hline - \tcode{vertex_id_t} & \tcode{remove_cvref_t} & \\ - \textit{\tcode{raw-vertex-id-type}} & \tcode{decltype(target_id(el,uv))} & exposition only \\ + \tcode{vertex_id_t} & \tcode{remove_cvref_t} & \\ + \textit{\tcode{raw-vertex-id-type}} & \tcode{decltype(source_id(el,uv))} & exposition only \\ \hline \end{tabular}} \caption{Edgelist Interface Type Aliases} diff --git a/agents/D3130_review.md b/agents/D3130_review.md index dd330ac..ae1b494 100644 --- a/agents/D3130_review.md +++ b/agents/D3130_review.md @@ -42,16 +42,16 @@ wording/snippet polish. `vertex_descriptor` and describe an `edge_descriptor` as an iterator plus the source `vertex_descriptor`. PDF rebuilds cleanly. -### 1.3 [MED] Edgelist `vertex_id_t`: paper says `target_id`, code uses `source_id` -- Paper: Table "Edgelist Interface Type Aliases" defines - `vertex_id_t = remove_cvref_t`, and the - Namespace section says the edgelist `vertex_id_t` "is derived ... from - `target_id(el,uv)`." -- Code: `edge_list/edge_list.hpp` derives both `vertex_id_t` and - `raw_vertex_id_t` from **`source_id(el,uv)`**. -- The two ids are constrained to be the same type, so this is harmless at - runtime, but the documentation should match the implementation (or vice - versa). Pick one consistently. +### 1.3 [MED] Edgelist `vertex_id_t`: paper said `target_id` — RESOLVED (paper aligned to `source_id`) +- Original mismatch: paper's type-alias table and namespace prose derived the + edgelist `vertex_id_t` from `target_id(el,uv)`; code derives both + `vertex_id_t` and `raw_vertex_id_t` from `source_id(el,uv)`. +- Resolution (paper updated to match code): the "Edgelist Interface Type + Aliases" table now shows `vertex_id_t = remove_cvref_t` + and `raw-vertex-id-type = decltype(source_id(el,uv))`; the namespace prose now + says the edgelist `vertex_id_t` is derived from `source_id(el,uv)`. (Both edge + ids represent vertices of the same set, so either is a valid representative; + `source_id` is chosen to match the implementation.) PDF rebuilds cleanly. ### 1.4 [MED] `vertex_id_store_t` does not exist - Paper: Table "Graph Container Interface Type Aliases" lists @@ -185,9 +185,8 @@ wording/snippet polish. - **Q3 (edgelist functions):** Are `num_edges(el)` / `has_edges(el)` / `contains_edge(el,uid,vid)` part of the proposed interface (document as-is) or should they be flagged "not yet implemented"? (See 1.7.) -- **Q4 (edgelist `vertex_id`):** Should the edgelist `vertex_id_t` be derived - from `source_id` (as the code does) or `target_id` (as the paper says)? (See - 1.3.) +- **Q4 (edgelist `vertex_id`):** RESOLVED — paper aligned to `source_id` (the + code's choice). See 1.3. - **Q5 (`hashable_vertex_id`):** Does the `hashable_vertex_id` concept belong in this paper (it is GCI-local and underpins `mapped_vertex_range`), or is it intentionally hidden? (See 1.9.) From edaf8d49b8fdd0fd9470af5a2e75ed9614b8fd73 Mon Sep 17 00:00:00 2001 From: Phil Ratzloff Date: Mon, 8 Jun 2026 22:39:59 -0400 Subject: [PATCH 5/9] D3130: remove phantom vertex_id_store_t type alias vertex_id_store_t was listed as 'normative' with an undefined '(see below)' definition but has no counterpart in the implementation. Remove it from the type-alias table and the revision history; retain the valid raw-vertex-id-type exposition-only alias (raw_vertex_id_t in code). Resolves review item 1.4. --- .../tex/container_interface.tex | 1 - D3130_Container_Interface/tex/revision.tex | 2 +- agents/D3130_review.md | 15 ++++++++------- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/D3130_Container_Interface/tex/container_interface.tex b/D3130_Container_Interface/tex/container_interface.tex index 3ff8d99..ce59a6f 100644 --- a/D3130_Container_Interface/tex/container_interface.tex +++ b/D3130_Container_Interface/tex/container_interface.tex @@ -180,7 +180,6 @@ \subsection{Types} \tcode{vertex_iterator_t} & \tcode{iterator_t>} & \\ \tcode{vertex_t} & \tcode{range_value_t>} & descriptor \\ \tcode{vertex_id_t} & \tcode{remove_cvref_t))>} & \\ - \tcode{vertex_id_store_t} & (see below) & normative \\ \textit{\tcode{raw-vertex-id-type}} & \tcode{decltype(vertex_id(g,vertex_t))} & exposition only \\ \tcode{vertex_value_t} & \tcode{decltype(vertex_value(g,u))} & optional \\ \hline diff --git a/D3130_Container_Interface/tex/revision.tex b/D3130_Container_Interface/tex/revision.tex index 78da8e0..35a2015 100644 --- a/D3130_Container_Interface/tex/revision.tex +++ b/D3130_Container_Interface/tex/revision.tex @@ -79,7 +79,7 @@ \subsection*{\paperno r4} graphs with non-integer or string vertex ids. Related changes: \begin{itemize} \item New concept \tcode{mapped_vertex_range} alongside \tcode{index_vertex_range}. - \item New type aliases \tcode{vertex_id_store_t} and \textit{raw-vertex-id-type}. + \item New \textit{raw-vertex-id-type} exposition-only type alias. \item New \tcode{vertex_property_map} utility type alias; \tcode{make_vertex_property_map} (eager and lazy), \tcode{vertex_property_map_contains}, \tcode{vertex_property_map_get}. diff --git a/agents/D3130_review.md b/agents/D3130_review.md index ae1b494..d6dc829 100644 --- a/agents/D3130_review.md +++ b/agents/D3130_review.md @@ -53,13 +53,14 @@ wording/snippet polish. ids represent vertices of the same set, so either is a valid representative; `source_id` is chosen to match the implementation.) PDF rebuilds cleanly. -### 1.4 [MED] `vertex_id_store_t` does not exist -- Paper: Table "Graph Container Interface Type Aliases" lists - `vertex_id_store_t` and labels it "normative". -- Code: no such alias. The closest is `raw_vertex_id_t` (the paper's - "*raw-vertex-id-type*", exposition only). -- Action: remove `vertex_id_store_t` or replace with the actual alias name - and definition. +### 1.4 [MED] `vertex_id_store_t` does not exist — RESOLVED (removed from paper) +- Original mismatch: the type-alias table listed `vertex_id_store_t` (labeled + "normative") with a "(see below)" definition that was never actually defined; + no such alias exists in the code. +- Resolution (paper updated): removed the `vertex_id_store_t` row from the + "Graph Container Interface Type Aliases" table and the stale mention in + `revision.tex`. The valid `raw-vertex-id-type` exposition-only alias (which + maps to the code's `raw_vertex_id_t`) is retained. PDF rebuilds cleanly. ### 1.5 [MED] Adjacency-matrix trait family is undocumented-as-future and partly absent - Paper: traits table lists `define_adjacency_matrix` (flagged "Not yet in From 66b2029ec0d2c7e84eee83c3df8d7949bbfaaf0b Mon Sep 17 00:00:00 2001 From: Phil Ratzloff Date: Mon, 8 Jun 2026 22:44:56 -0400 Subject: [PATCH 6/9] D3130: flag unimplemented features as 'not yet in reference implementation' Mark the adjacency_matrix trait family and its constant-time contains_out_edge branch (1.5), the partition-filtered out_edges(g,*,pid) overloads (1.6), and the edgelist num_edges/has_edges/contains_edge functions (1.7) as future design not yet in the reference implementation. Resolves review items 1.5-1.7 / Q1-Q3. --- .../tex/container_interface.tex | 12 ++- agents/D3130_review.md | 73 +++++++++---------- 2 files changed, 45 insertions(+), 40 deletions(-) diff --git a/D3130_Container_Interface/tex/container_interface.tex b/D3130_Container_Interface/tex/container_interface.tex index ce59a6f..455ecec 100644 --- a/D3130_Container_Interface/tex/container_interface.tex +++ b/D3130_Container_Interface/tex/container_interface.tex @@ -146,10 +146,11 @@ \subsection{Traits} \tcode{has_basic_queries} & concept & \tcode{has_degree \&\& has_find_vertex \&\& has_find_vertex_edge} \\ \tcode{has_full_queries} & concept & \tcode{has_basic_queries \&\& has_contains_edge>} \\ \hline - \tcode{define_adjacency_matrix : false_type} & struct & Specialize for graph implementation to derive from \tcode{true_type} for edges stored as a square 2-dimensional array. Not yet in reference implementation. \\ + \tcode{define_adjacency_matrix : false_type} & struct & Specialize for graph implementation to derive from \tcode{true_type} for edges stored as a square 2-dimensional array. \\ \tcode{is_adjacency_matrix} & struct & \\ \tcode{is_adjacency_matrix_v} & type alias & \\ \tcode{adjacency_matrix} & concept & \\ + \multicolumn{3}{l}{\emph{The adjacency\_matrix trait family is not yet in the reference implementation.}} \\ \hline \end{tabular}} \caption{Graph Container Interface Type Traits} @@ -485,6 +486,10 @@ \subsection{Functions} Functions that have n/a for their Default Implementation must be defined by the author of a Graph Container implementation. +The constant-time \tcode{contains_out_edge(g,uid,vid)} branch guarded by \tcode{is_adjacency_matrix_v} depends on the +adjacency\_matrix trait family, which is not yet in the reference implementation; until then the linear \tcode{find_out_edge}-based +default is always used. + Value functions (\tcode{graph_value(g)}, \tcode{vertex_value(g,u)} and \tcode{edge_value(g,uv)}) can be optionally implemented, depending on whether the graph container supports values on the graph, vertex and edge types. They return a single value and can be scalar, struct, class, union, or tuple. These are abstract types used by the GVF, VVF and EVF function objects to retrieve @@ -626,7 +631,7 @@ \subsection{Unipartite, Bipartite and Multipartite Graph Representation} \end{itemize} \tcode{out_edges(g,uid,pid)} and \tcode{out_edges(g,u,pid)} filter the edges where the target is in the partition \tcode{pid} passed. -This isn't needed for bipartite graphs. +This isn't needed for bipartite graphs. These partition-filtered \tcode{out_edges} overloads are not yet in the reference implementation. \section{Edgelist Interface} An edgelist is a range of values where we can get the source\_id and target\_id, and an optional edge\_value. It is similar to edges in an @@ -748,6 +753,9 @@ \subsection{Functions} \end{center} \end{table} +The \tcode{contains_edge(el,uid,vid)}, \tcode{num_edges(el)} and \tcode{has_edges(el)} functions are not yet in the reference +implementation; the \tcode{source_id}, \tcode{target_id} and \tcode{edge_value} CPOs are available now. + % \tcode{find_if(el, )} % [](edge_reference_t) \{return source_id(e)==uid && target_id(e)==vid\} diff --git a/agents/D3130_review.md b/agents/D3130_review.md index d6dc829..4ea5040 100644 --- a/agents/D3130_review.md +++ b/agents/D3130_review.md @@ -62,35 +62,34 @@ wording/snippet polish. `revision.tex`. The valid `raw-vertex-id-type` exposition-only alias (which maps to the code's `raw_vertex_id_t`) is retained. PDF rebuilds cleanly. -### 1.5 [MED] Adjacency-matrix trait family is undocumented-as-future and partly absent -- Paper: traits table lists `define_adjacency_matrix` (flagged "Not yet in - reference implementation"), plus `is_adjacency_matrix`, `is_adjacency_matrix_v`, - and the `adjacency_matrix` concept (no flag). The `contains_out_edge` default - in Table "Edge Functions" describes a constant-time path guarded by - `is_adjacency_matrix_v`. -- Code: none of `define_adjacency_matrix`, `is_adjacency_matrix`, - `is_adjacency_matrix_v`, or `adjacency_matrix` exist. The actual - `contains_edge`/`contains_out_edge` default always does a linear `find_if` — - there is no adjacency-matrix O(1) shortcut. -- Action: either (a) mark the whole family + the `contains_out_edge` constant - branch as "not yet in reference implementation", or (b) drop them. See Open - Question Q1. - -### 1.6 [MED] Partition-filtered `out_edges(g,*,pid)` not implemented -- Paper: the partitions section states `out_edges(g,uid,pid)` and - `out_edges(g,u,pid)` "filter the edges where the target is in the partition - `pid`." -- Code: no `out_edges` overload takes a `pid`. Only `vertices(g,pid)`, - `num_vertices(g,pid)`, `num_partitions(g)`, and `partition_id(g,u)` exist. -- Action: mark as future, or remove. See Open Question Q2. - -### 1.7 [MED] Edgelist functions `num_edges/has_edges/contains_edge` are TODO in code -- Paper: Table "Edgelist Interface Functions" lists `num_edges(el)`, - `has_edges(el)`, and `contains_edge(el,uid,vid)` with default implementations. -- Code: `edge_list/edge_list.hpp` header marks these "(todo)"; only - `source_id`, `target_id`, `edge_value` CPOs are implemented in `edge_cpo.hpp`. -- Action: confirm intent — keep documented as the target interface, or add a - "not yet implemented" note. See Open Question Q3. +### 1.5 [MED] Adjacency-matrix trait family — RESOLVED (flagged not-yet-implemented) +- Paper: traits table listed `define_adjacency_matrix`, `is_adjacency_matrix`, + `is_adjacency_matrix_v`, and the `adjacency_matrix` concept; the + `contains_out_edge` default described a constant-time path guarded by + `is_adjacency_matrix_v`. None exist in the code; the actual default always + does a linear `find_if`. +- Resolution (paper updated): added a group note to the traits table marking the + whole adjacency\_matrix family as "not yet in the reference implementation," + and a note after the Edge Functions table that the constant-time + `contains_out_edge` branch is likewise future (the linear default is always + used until then). Kept as documented future design per author direction. + +### 1.6 [MED] Partition-filtered `out_edges(g,*,pid)` — RESOLVED (flagged not-yet-implemented) +- Paper stated `out_edges(g,uid,pid)` / `out_edges(g,u,pid)` filter edges by + target partition; no such overload exists in the code (only `vertices(g,pid)`, + `num_vertices(g,pid)`, `num_partitions(g)`, `partition_id(g,u)`). +- Resolution (paper updated): the partitions section now states these + partition-filtered `out_edges` overloads are not yet in the reference + implementation. Kept as documented future API per author direction. + +### 1.7 [MED] Edgelist functions `num_edges/has_edges/contains_edge` — RESOLVED (flagged not-yet-implemented) +- Paper documented `num_edges(el)`, `has_edges(el)`, `contains_edge(el,uid,vid)` + with default implementations; the code marks these "(todo)" and implements + only `source_id`/`target_id`/`edge_value`. +- Resolution (paper updated): added a note after the Edgelist Interface + Functions table that these three functions are not yet in the reference + implementation while the id/value CPOs are available now. Kept as documented + target interface per author direction. ### 1.8 [LOW] `vertex_value(g,uid)` convenience overload missing in code - Paper: vertex-functions table lists `vertex_value(g,uid)` with default @@ -177,15 +176,13 @@ wording/snippet polish. ## 4. Open questions (scope / intent — need author input) -- **Q1 (adjacency matrix):** Should the adjacency-matrix trait family - (`define_adjacency_matrix`, `is_adjacency_matrix`, `is_adjacency_matrix_v`, - `adjacency_matrix`) and the constant-time `contains_out_edge` branch stay in - this paper as *future design*, or be removed until implemented? (See 1.5.) -- **Q2 (partition edge filtering):** Keep `out_edges(g,uid,pid)` / - `out_edges(g,u,pid)` as documented future API, or remove? (See 1.6.) -- **Q3 (edgelist functions):** Are `num_edges(el)` / `has_edges(el)` / - `contains_edge(el,uid,vid)` part of the proposed interface (document as-is) or - should they be flagged "not yet implemented"? (See 1.7.) +- **Q1 (adjacency matrix):** RESOLVED — kept as future design, flagged "not yet + in the reference implementation" in the traits table and Edge Functions note. + See 1.5. +- **Q2 (partition edge filtering):** RESOLVED — kept as future API, flagged not + yet implemented in the partitions section. See 1.6. +- **Q3 (edgelist functions):** RESOLVED — kept as target interface, flagged not + yet implemented after the Edgelist Functions table. See 1.7. - **Q4 (edgelist `vertex_id`):** RESOLVED — paper aligned to `source_id` (the code's choice). See 1.3. - **Q5 (`hashable_vertex_id`):** Does the `hashable_vertex_id` concept belong in From 01f6e90f472373015a70d6ae49af470e60e62fcf Mon Sep 17 00:00:00 2001 From: Phil Ratzloff Date: Mon, 8 Jun 2026 22:51:22 -0400 Subject: [PATCH 7/9] D3130 review: mark 1.8 resolved (vertex_value(g,uid) added to code) --- agents/D3130_review.md | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/agents/D3130_review.md b/agents/D3130_review.md index 4ea5040..a4a1937 100644 --- a/agents/D3130_review.md +++ b/agents/D3130_review.md @@ -91,12 +91,16 @@ wording/snippet polish. implementation while the id/value CPOs are available now. Kept as documented target interface per author direction. -### 1.8 [LOW] `vertex_value(g,uid)` convenience overload missing in code -- Paper: vertex-functions table lists `vertex_value(g,uid)` with default - `vertex_value(g,*find_vertex(g,uid))`. -- Code: `_vertex_value` CPO provides only `operator()(g, u)` (descriptor form); - no `uid` overload. -- Action: either add the overload to the impl or footnote it as not-yet-provided. +### 1.8 [LOW] `vertex_value(g,uid)` convenience overload — RESOLVED (added to code) +- Original mismatch: paper's vertex-functions table documented + `vertex_value(g,uid)` (default `vertex_value(g,*find_vertex(g,uid))`); the + `_vertex_value` CPO provided only the descriptor form `operator()(g, u)`. +- Resolution (code updated to match paper): added a `uid` convenience overload + to the `_vertex_value` CPO that resolves to + `(*this)(g, *find_vertex(g, uid))`, gated by a `_has_uid` constraint (uid is + not a vertex descriptor, `find_vertex(g,uid)` is valid, and `vertex_value` is + available on the resulting descriptor). Added a focused uid-overload test; + adj_list (now 26 vertex_value cases) and container suites pass. ### 1.9 [LOW] `mapped_vertex_range` snippet omits the `hashable_vertex_id` requirement - Paper: `src/concepts_vertex_range.hpp` shows `mapped_vertex_range` as From 8afce3843125bd09faa19f475c487107cc85677a Mon Sep 17 00:00:00 2001 From: Phil Ratzloff Date: Mon, 8 Jun 2026 23:00:59 -0400 Subject: [PATCH 8/9] D3130 review: note improved vertex_value(g,uid) dispatch logic --- agents/D3130_review.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/agents/D3130_review.md b/agents/D3130_review.md index a4a1937..6ce50ab 100644 --- a/agents/D3130_review.md +++ b/agents/D3130_review.md @@ -96,11 +96,12 @@ wording/snippet polish. `vertex_value(g,uid)` (default `vertex_value(g,*find_vertex(g,uid))`); the `_vertex_value` CPO provided only the descriptor form `operator()(g, u)`. - Resolution (code updated to match paper): added a `uid` convenience overload - to the `_vertex_value` CPO that resolves to - `(*this)(g, *find_vertex(g, uid))`, gated by a `_has_uid` constraint (uid is - not a vertex descriptor, `find_vertex(g,uid)` is valid, and `vertex_value` is - available on the resulting descriptor). Added a focused uid-overload test; - adj_list (now 26 vertex_value cases) and container suites pass. + to the `_vertex_value` CPO. It mirrors the descriptor dispatch — preferring a + member `g.vertex_value(uid)` or ADL `vertex_value(g, uid)` that takes the id + directly, and only falling back to `vertex_value(g, *find_vertex(g, uid))` + when neither exists — via a dedicated `_Choose_uid` strategy. Added tests for + the find_vertex fallback and for id-taking member priority; adj_list (27 + vertex_value cases) and container suites pass. ### 1.9 [LOW] `mapped_vertex_range` snippet omits the `hashable_vertex_id` requirement - Paper: `src/concepts_vertex_range.hpp` shows `mapped_vertex_range` as From c99753a88cff47ce6a67c75fbb52a0bdf2976537 Mon Sep 17 00:00:00 2001 From: Phil Ratzloff Date: Mon, 8 Jun 2026 23:06:14 -0400 Subject: [PATCH 9/9] D3130: document hashable_vertex_id, vertex descriptor guard, partition defaults - Add hashable_vertex_id concept and include it in mapped_vertex_range (1.9/Q5) - Add is_vertex_descriptor_v guard to the vertex concept snippet + prose (1.10) - Clarify partition vertices(g,pid)/num_vertices(g,pid) defaults are pid==0 only (2.1) - Document num_edges(g,u)/num_edges(g,uid) as aliases for out_degree (2.2) Resolves review items 1.9, 1.10, 2.1, 2.2, 2.3 / Q5. --- .../src/concepts_vertex_range.hpp | 16 +++- .../tex/container_interface.tex | 15 +++- agents/D3130_review.md | 75 ++++++++++--------- 3 files changed, 61 insertions(+), 45 deletions(-) diff --git a/D3130_Container_Interface/src/concepts_vertex_range.hpp b/D3130_Container_Interface/src/concepts_vertex_range.hpp index e62f0c6..ccee44d 100644 --- a/D3130_Container_Interface/src/concepts_vertex_range.hpp +++ b/D3130_Container_Interface/src/concepts_vertex_range.hpp @@ -1,10 +1,11 @@ // For exposition only template -concept vertex = requires(G& g, const V& u, const vertex_id_t& uid) { - vertex_id(g, u); - find_vertex(g, uid); -}; +concept vertex = is_vertex_descriptor_v> && + requires(G& g, const V& u, const vertex_id_t& uid) { + vertex_id(g, u); + find_vertex(g, uid); + }; template concept vertex_range = forward_range && @@ -19,9 +20,16 @@ concept index_vertex_range = { vertices(g) } -> vertex_range; }; +// vertex id usable as an unordered\_map key (for mapped property maps) +template +concept hashable_vertex_id = requires(const vertex_id_t& uid) { + { hash>{}(uid) } -> convertible_to; +}; + template concept mapped_vertex_range = !index_vertex_range && + hashable_vertex_id && requires(G& g, const vertex_id_t& uid) { { vertices(g) } -> forward_range; find_vertex(g, uid); diff --git a/D3130_Container_Interface/tex/container_interface.tex b/D3130_Container_Interface/tex/container_interface.tex index 455ecec..fd7ad0d 100644 --- a/D3130_Container_Interface/tex/container_interface.tex +++ b/D3130_Container_Interface/tex/container_interface.tex @@ -93,14 +93,20 @@ \subsubsection{Edge Concepts} } \subsubsection{Vertex Concepts} -The \tcode{vertex} concept requires that a vertex supports \tcode{vertex_id(g,u)} and \tcode{find_vertex(g,uid)}. +The \tcode{vertex} concept requires that \tcode{V} is a vertex descriptor (\tcode{is_vertex_descriptor_v}) +that supports \tcode{vertex_id(g,u)} and \tcode{find_vertex(g,uid)}. The descriptor guard distinguishes a +vertex descriptor from a raw vertex value, so the concept does not accidentally match unrelated types. Building on it, \tcode{vertex_range} requires a \tcode{forward_range} and \tcode{sized_range} whose value type satisfies \tcode{vertex}. +The \tcode{hashable_vertex_id} concept holds when \tcode{vertex_id_t} is usable as a key in a +\tcode{std::unordered_map} (i.e.\ it is hashable). It is used by \tcode{mapped_vertex_range} and by the +\tcode{vertex_property_map} utilities, which back per-vertex state with an \tcode{unordered_map} for mapped graphs. + Two specializations of vertex range are provided: \begin{itemize} \item \tcode{index_vertex_range} — vertex id is integral and the vertex range is random-access with an integral \tcode{storage_type}. Used for high-performance graphs such as \tcode{index_adjacency_list}. - \item \tcode{mapped_vertex_range} — vertex id is non-integral and vertices are found via \tcode{find_vertex(g, uid)}. Used for graphs whose vertices are stored in associative containers. + \item \tcode{mapped_vertex_range} — not an \tcode{index_vertex_range}, the vertex id is \tcode{hashable_vertex_id}, and vertices are found via \tcode{find_vertex(g, uid)}. Used for graphs whose vertices are stored in associative containers. \end{itemize} {\small \lstinputlisting{D3130_Container_Interface/src/concepts_vertex_range.hpp} @@ -378,8 +384,8 @@ \subsection{Functions} \tcode{has_edges(g)} & \tcode{bool} & |V| & \tcode{for(u: vertices(g)) if !empty(out\_edges(g,u)) return true; return false;} \\ \hdashline \tcode{num_partitions(g)} & \tcode{integral} & constant & 1 \\ - \tcode{vertices(g,pid)} & \tcode{partition_vertex_range_t} & constant & \tcode{vertices(g)} \\ - \tcode{num_vertices(g,pid)} & \tcode{integral} & constant & \tcode{size(vertices(g))} \\ + \tcode{vertices(g,pid)} & \tcode{partition_vertex_range_t} & constant & \tcode{vertices(g)} if \tcode{pid==0}, else empty range \\ + \tcode{num_vertices(g,pid)} & \tcode{integral} & constant & \tcode{size(vertices(g))} if \tcode{pid==0}, else 0 \\ \hline \end{tabular}} \caption{Graph Functions} @@ -417,6 +423,7 @@ \subsection{Functions} \tcode{out_degree(g,uid)} & \tcode{integral} & constant & \tcode{size(out\_edges(g,uid))} if \tcode{sized_range>} \\ (\tcode{edges(g,u), edges(g,uid)} & & & aliases for \tcode{out\_edges}) \\ (\tcode{degree(g,u), degree(g,uid)} & & & aliases for \tcode{out\_degree}) \\ + (\tcode{num\_edges(g,u), num\_edges(g,uid)} & & & aliases for \tcode{out\_degree}) \\ \hdashline \tcode{in_edges(g,u)} & \tcode{in_edge_range_t} & constant & n/a (requires bidirectional graph) \\ \tcode{in_edges(g,uid)} & \tcode{in_edge_range_t} & constant & \tcode{in\_edges(g,*find\_vertex(g,uid))} \\ diff --git a/agents/D3130_review.md b/agents/D3130_review.md index 6ce50ab..f771b66 100644 --- a/agents/D3130_review.md +++ b/agents/D3130_review.md @@ -103,48 +103,49 @@ wording/snippet polish. the find_vertex fallback and for id-taking member priority; adj_list (27 vertex_value cases) and container suites pass. -### 1.9 [LOW] `mapped_vertex_range` snippet omits the `hashable_vertex_id` requirement -- Paper: `src/concepts_vertex_range.hpp` shows `mapped_vertex_range` as - `!index_vertex_range && requires { vertices(g) -> forward_range; find_vertex(g,uid); }`. -- Code: also requires `hashable_vertex_id` (vertex id must be usable as an - `unordered_map` key). The `hashable_vertex_id` concept is not mentioned in the - paper at all. -- Action: add `hashable_vertex_id` to the snippet and a one-line description, - or note why it is intentionally elided. - -### 1.10 [LOW] `vertex` snippet omits the `is_vertex_descriptor_v` guard -- Paper: `src/concepts_vertex_range.hpp` `vertex` concept requires - `vertex_id(g,u)` and `find_vertex(g,uid)`. -- Code: additionally requires `is_vertex_descriptor_v>`. -- Action: add the guard to the snippet (minor, but it is load-bearing for - disambiguating descriptors from raw values). +### 1.9 [LOW] `mapped_vertex_range` missing `hashable_vertex_id` — RESOLVED (documented in paper) +- Original gap: the paper's `mapped_vertex_range` snippet omitted the + `hashable_vertex_id` requirement present in the code, and the + `hashable_vertex_id` concept was not mentioned at all. +- Resolution (paper updated): `src/concepts_vertex_range.hpp` now defines + `hashable_vertex_id` and includes it in `mapped_vertex_range`; the Vertex + Concepts prose introduces the concept (vertex id usable as an `unordered_map` + key, used by `mapped_vertex_range` and the `vertex_property_map` utilities) and + updates the `mapped_vertex_range` bullet to mention it. PDF rebuilds cleanly. + +### 1.10 [LOW] `vertex` missing `is_vertex_descriptor_v` guard — RESOLVED (added to snippet) +- Original gap: the paper's `vertex` snippet omitted the + `is_vertex_descriptor_v>` guard present in the code. +- Resolution (paper updated): the `vertex` concept snippet now leads with the + descriptor guard, and the Vertex Concepts prose explains it distinguishes a + vertex descriptor from a raw vertex value so the concept does not accidentally + match unrelated types. PDF rebuilds cleanly. --- ## 2. Clarity / completeness observations -### 2.1 [LOW] Partition default-implementation semantics understated -- Paper defaults: `vertices(g,pid)` → `vertices(g)`; `num_vertices(g,pid)` → - `size(vertices(g))`. -- Code defaults: return the full range/count **only when `pid==0`**, otherwise - an empty range / `0`. This matches the "one partition holds everything" model - but the table omits the `pid==0` condition, which could mislead implementers. -- Action: state the defaults as "for `pid==0`, returns all vertices; otherwise - empty / 0". - -### 2.2 [LOW] Undocumented `num_edges(g,u)` overload -- Code has a `num_edges(g, u)` (per-vertex) CPO in addition to `num_edges(g)`. - The paper documents only `num_edges(g)` (and `out_degree`). If `num_edges(g,u)` - is intended as public API, document it or alias it to `out_degree`; if it is - internal, no change needed. - -### 2.3 Concept count wording +### 2.1 [LOW] Partition default-implementation semantics — RESOLVED (paper clarified) +- Original gap: the Graph Functions table gave the `vertices(g,pid)` / + `num_vertices(g,pid)` defaults as `vertices(g)` / `size(vertices(g))` without + the `pid==0` condition the code applies (empty range / 0 otherwise). +- Resolution (paper updated): the table now reads `vertices(g)` if `pid==0` + else empty range, and `size(vertices(g))` if `pid==0` else 0. PDF rebuilds + cleanly. + +### 2.2 [LOW] Undocumented `num_edges(g,u)` overload — RESOLVED (documented as alias) +- Original gap: the code has per-vertex `num_edges(g,u)` / `num_edges(g,uid)` + CPOs (same result as `out_degree`) that the paper did not document. +- Resolution (paper updated): added an alias line to the Vertex Functions table + noting `num_edges(g,u), num_edges(g,uid)` are aliases for `out_degree`, + alongside the existing `edges`/`degree` alias rows. PDF rebuilds cleanly. + +### 2.3 Concept count wording — RESOLVED - Paper: "Six concepts are defined" for the adjacency list — accurate for the documented set. Code additionally defines `ordered_vertex_edges` (used by triangle-count in the algorithms paper) and `hashable_vertex_id` (used by - `mapped_vertex_range`). `ordered_vertex_edges` is reasonably left to the - algorithms paper; `hashable_vertex_id` is GCI-local and probably belongs here - (see 1.9). + `mapped_vertex_range`). `ordered_vertex_edges` is left to the algorithms paper; + `hashable_vertex_id` is now documented in this paper (see 1.9). --- @@ -190,6 +191,6 @@ wording/snippet polish. yet implemented after the Edgelist Functions table. See 1.7. - **Q4 (edgelist `vertex_id`):** RESOLVED — paper aligned to `source_id` (the code's choice). See 1.3. -- **Q5 (`hashable_vertex_id`):** Does the `hashable_vertex_id` concept belong in - this paper (it is GCI-local and underpins `mapped_vertex_range`), or is it - intentionally hidden? (See 1.9.) +- **Q5 (`hashable_vertex_id`):** RESOLVED — documented in this paper; it is + GCI-local and underpins `mapped_vertex_range` and the `vertex_property_map` + utilities. See 1.9.