Skip to content

Conversation

@harrysroberts
Copy link

@harrysroberts harrysroberts commented Oct 21, 2025

This change adds additional functionality to the dodgr_components() function, where a new Boolean flag strong will cause the function to find the strong components of a directed graph. Thus, the partition of the graph into maximal subgraphs in which, for each pair of vertices i and j, paths exist both from i to j and from j to i. This is discussed further in Issue #322 .

The main addition is to graph.cpp, in which new functions identify_graph_strong_components and its recursive helper strong_connect implement Tarjan's strongly connected components algorithm.

The format of the output of this algorithm, in comparison to the weak version, necessitated some minor adjustments to higher-level functions.

Firstly, within rcpp_get_component_vector, the mapping of component numbers to edges has been adjusted such that an edge is always assigned the component of its start node.

for (auto e : edge_map)
{
    vertex_id_t vi = e.second.get_from_vertex ();
    comp_nums.emplace (e.first, components.find(vi)->second);
}

For the weak algorithm, this won't have any effect, as both start and end node are necessarily in the same component. However, for the strong algorithm this is not always the case, so edges that cross between strong components are (arbitrarily) assigned consistently to that of the start node.

Secondly, within the R function dodgr_components, the assignment of component numbers in descending order is adjusted to use the frequencies when sorting.

freqs <- table (component)
sorted_components <- names (freqs) [order (freqs, decreasing = TRUE)]
graph$component <- match (as.character (component), sorted_components)

As the strong algorithm may result in components with no edges (i.e. individual nodes with no outbound edges), some numbers will be missed in the component table, resulting in an incorrect assignment of components. Using frequencies resolves this error.

Other changes to graph-functions.R, graph.h and graph.cpp were to insert the Boolean flag strong to the necessary functions, ensuring that it defaults to FALSE so as to be compatible with existing code.

@mpadge
Copy link
Member

mpadge commented Oct 23, 2025

Thanks so much @harrysroberts , this looks great. I'll be back in my office next week, and will give it a run then.

Just one comment in the meantime: Lots of lines in the PR are just formating/lint modifications. Could you please revert these, to remove all the line changes like changing fn (x) to fn(x) - removing the space before the parenthesis. I know it's inconvenient for some, but dodgr uses the spaceout package for formtting and linting. If you configure your linter to use that instead of default, it should revert all of these changes. (And these days, a big motivation for me keeping this non-standard style is to help me retain control in a world of ever-increasing default behaviour.)

@harrysroberts
Copy link
Author

Hi @mpadge , I've made those formatting adjustments, I thought I'd spotted all of those but I suppose it does it automatically if you don't have spaceout

@mpadge
Copy link
Member

mpadge commented Oct 31, 2025

@harrysroberts Checks are failing because you need to update the docs with the new strong parameter. Just make doc in this directory, or devtools::document() in an R session.

@codecov
Copy link

codecov bot commented Oct 31, 2025

Codecov Report

❌ Patch coverage is 18.51852% with 44 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.01%. Comparing base (6775463) to head (a7747f1).

Files with missing lines Patch % Lines
src/graph.cpp 12.00% 44 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #324      +/-   ##
==========================================
- Coverage   93.71%   93.01%   -0.71%     
==========================================
  Files          52       52              
  Lines        6940     6768     -172     
==========================================
- Hits         6504     6295     -209     
- Misses        436      473      +37     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mpadge
Copy link
Member

mpadge commented Oct 31, 2025

@harrysroberts Can you also please:

  • Add yourself to DESC as an "aut", because you've authored an entire function. Once you've done that, please also run codemetar::write_codemeta(), to update the metadata with you as new author.
  • Update NEWS.md with two more items, one describing this, and one stating that you've been added as a new author.

Thanks!

@harrysroberts
Copy link
Author

@mpadge Done as requested

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