Skip to content

Node order#17

Closed
aj2duncan wants to merge 2 commits into
iankloo:developmentfrom
aj2duncan:node_order
Closed

Node order#17
aj2duncan wants to merge 2 commits into
iankloo:developmentfrom
aj2duncan:node_order

Conversation

@aj2duncan

Copy link
Copy Markdown
Contributor

What my pull fixes (if applicable)

#15

Confirmation that you have tested your code

Example of test code below, although I have also used other graphs. There can be small rounding errors but I haven't had any mismatches down to 10^(-12).

library(sigmaNet)
library(igraph)

data = lesMis

sigmaObj = sigmaFromIgraph(data)

# code from addNodeSize()
edges <- jsonlite::fromJSON(sigmaObj$x$data)$edges
nodes <- jsonlite::fromJSON(sigmaObj$x$data)$nodes

tmp_graph <- igraph::graph_from_data_frame(sigmaObj$x$graph$edges,
                                           directed = FALSE)


nodes_size <- igraph::eigen_centrality(tmp_graph)$vector
nodes_order <- order(as.numeric(names(nodes_size)))

nodes$size <- nodes_size[nodes_order]

# there are some small rounding errors so identical() give FALSE
table(abs(igraph::eigen_centrality(data)$vector - nodes$size) < 1e-12)

@iankloo

iankloo commented May 6, 2018

Copy link
Copy Markdown
Owner

Nice catch! I should have anticipated that igraph's reliance on c++ compiled functions would create inconsistencies the way I was using it. I'll review these changes and get the merged ASAP. Thanks.

@aj2duncan

Copy link
Copy Markdown
Contributor Author

I realised a couple of things the other day:

  • If a node has no connections it won't be included in the calculation of any of the metrics (as the current method of recreating the graph only looks at edges)
  • There is an assumption in my PR that all the nodes have numerical names

I will see if I can fix the first issue quickly but I'm wondering if you have any thoughts about the second problem.

@aj2duncan

Copy link
Copy Markdown
Contributor Author

Found what I think is a neater solution so closing this PR.

@aj2duncan aj2duncan closed this May 17, 2018
@aj2duncan aj2duncan mentioned this pull request May 17, 2018
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