Repository navigation
Fix silently wrong participation coefficients - #3
Merged
Merged
Conversation
agallinat
commented
Oct 3, 2026
agallinat
commented
Owner
- Fix five runtime defects and add a testthat suite
- Prepare DESCRIPTION, LICENSE and .Rbuildignore for CRAN
- Make all examples executable
- Replace deprecated ggplot2 and igraph calls
- Extract shared graph validator and collapse duplicate diffusion kernel
- Add release infrastructure and fix documentation typos
- Do not claim untested platforms in cran-comments.md
- Adopt the .9000 development-version convention
- Stop setting a future plan inside network_diffusion_with_pvalues()
- Make the plot return slot uniform across the package
- Fix silently wrong participation coefficients in calculate_roles()
What looked like a coverage gap -- nodes missing from the roles table --
was also a correctness bug affecting the nodes that remained.
With communities = NULL, calculate_roles() delegated to find_modules() at
its default min_size = 3, which discards small modules. A node whose
module was discarded is absent from module_table, so membership[node] is
NA. In the participation-coefficient loop, table() silently drops those
NAs from a neighbour's module tally, but the denominator was still the
node's full degree. The fractions therefore summed to less than 1 and
P = 1 - sum(...) came out too high for any *retained* node with a
discarded neighbour.
Measured on a 150-node sample_gnp with cluster.method = "walktrap": 29
nodes absent, and of the 121 reported, 18 had a wrong P and 7 a wrong
role (e.g. P 0.75 instead of 0.50, R3 instead of R2).
Two fixes:
- find_modules() is now called with min_size = 1. Roles are a per-node
measure, so filtering modules by size here was never meaningful, and it
is what introduced the NAs. Every vertex now receives a role.
- The denominator is now the number of neighbours actually tallied rather
than the full degree. These agree whenever every neighbour has a module,
and the former stays well defined when some do not, so a partial
membership yields a defined P instead of an inflated one.
Partial coverage can still arise legitimately: spinglass cannot run on a
disconnected graph, so find_modules() restricts it to the largest
connected component. That case now warns with a count and example
vertices instead of returning a short table silently.
Two further latent defects on the user-supplied membership path, both
found while testing the above:
- A membership vector containing NAs errored with "Invalid vertex names":
NA was treated as a module and induced_subgraph() was handed NA node
names. NA modules are now skipped, and those vertices get role = NA.
- An *unnamed* membership vector of the correct length was accepted, then
had index names ("1", "2", ...) invented for it, so every subsequent
vertex lookup failed unless the graph's own names happened to match.
Unnamed vectors are now treated as positional, in vertex order, which is
what igraph::membership() returns.
Tests go from 371 to 386, including a regression test that compares the
find_modules() path against the equivalent explicit membership and
asserts identical P and roles.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.