CICD Updates for Igraph changes and Ubuntu Tidy patch

[bburns632]

This PR gets CICD tests back to working status after some updates in rigraph and Ubuntu images caused job failures:

• igraph updated their calculation of authorityScore in igraph release 0.10.5. Consequently, some expected values in tests needed to change.
• rigraph deprecated get.edgelist and get.vertex.attribute functions.
• added explicit Tidy-html5 import to github actions to resolve warning in R CMD Check on ubuntu. Resolves NOTE on Missing Tidy package causing Ubuntu jobs to fail #305

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.60%. Comparing base (9354e79) to head (25f6573).

❗ Current head 25f6573 differs from pull request most recent head 399a4b3. Consider uploading reports for the commit 399a4b3 to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #306   +/-   ##
=======================================
  Coverage   92.60%   92.60%           
=======================================
  Files          12       12           
  Lines         946      946           
=======================================
  Hits          876      876           
  Misses         70       70           

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

[jameslamb]

Thanks @bburns632 !

There's one part of this I don't understand though... shouldn't there also be some changes in R/ and man/?

Assuming this work was triggered by the NOTEs I see at https://cran.r-project.org/web/checks/check_results_pkgnet.html

Result: NOTE 
  checkRd: (-1) DependencyReporter.Rd:21-28: Lost braces in \itemize; meant \describe ?
  checkRd: (-1) DependencyReporter.Rd:44-55: Lost braces in \itemize; meant ...
  checkRd: (-1) PackageReport.Rd:50-52: Lost braces
      50 |                 \item{\bold{\code{reporter}}: Instantiated package reporter
         |                      ^
  checkRd: (-1) PackageReport.Rd:54-58: Lost braces in \itemize; meant \describe ?
  ...

and

Result: NOTE 
  Found the following HTML validation problems:
  DependencyReporter.html:35:1 (DependencyReporter.Rd:18): Warning: trimming empty <dt>
  FunctionReporter.html:101:1 (FunctionReporter.Rd:60): Warning: trimming empty <dt>
  InheritanceReporter.html:79:1 (InheritanceReporter.Rd:46): Warning: trimming empty <dt>
  PackageReporters.html:34:1 (PackageReporters.Rd:15): Warning: trimming empty <dt>
  SummaryReporter.html:33:1 (SummaryReporter.Rd:16): Warning: trimming empty <dt>

If adding tidy was all it took to reproduce those NOTEs from CRAN, I'd expect CI to be failing still because this PR adds it but doesn't modify any of the docs.

[bburns632]

That's a fair callout @jameslamb. I think we need to fix either roxygen or pkgdown to earlier versions (preferable to overhauling all our docs). Have any suggestions on versions to tie down in DESCRIPTION file?

[jameslamb]

I think it'd be better to use the latest version... it's hard to keep an == pin or ceiling working on CRAN indefinitely.

I could do it in a follow-up PR some time in the next few days if you'd like!

[bburns632]

I got some time. I'll take a look. Trying out this hack first: r-lib/roxygen2#457 (comment)

[bburns632]

OK. @jameslamb How about this? We just add Roxygen: list(r6 = FALSE) to the DESCRIPTION file to suppress the new formatting. I can't test on CRAN without resubmitting a new build (and I don't think this issue necessitates that), but this should resolve the issue. Roxygen is formatting our docs into a technically correct Rd that CRAN doesn't like. This is likely because we are not following the new format for R6 object docs (as we wrote them way before that). With this bit of code, it should format as it did before, with which CRAN had no issues.

I checked out the pkgdown build locally (mac) and it looked to be formatted fine with this parameter.

Let's let it fly.

[jameslamb]

Oh yeah I'm good with that! Ship it.

[bburns632]

@jameslamb just need you to hit the approve button, and it'll be shipped. Blocked until then as I tagged you as a reviewer.

[jameslamb]

ah sorry about that, approved!

[szhorvat]

• igraph updated their calculation of authorityScore in igraph release 0.10.5. Consequently, some expected values in tests needed to change.

Could you please clarify if you are seeing any change in results when computing the authority score of a directed graph? The fix in igraph 0.10.5 affected only undirected graphs (where hub and authority scores are identical to each other, as well as to the eigenvector centrality).

If you see a change on directed graphs, something may be wrong.

[bburns632]

@szhorvat , I would appreciate your help investigating this potential issue. It is an directed graph.

Here is where we calculated the authority_score for a Directed Graph via igraph:

pkgnet/R/GraphClasses.R

Lines 462 to 468 in a904eca

	# Authority Score
	, authorityScore = function(self){
	igraph::authority_score(
	graph = self$igraph
	, scale = TRUE
	)$vector
	}

Here is where we build the iGraph object:

pkgnet/R/GraphClasses.R

Lines 251 to 281 in a904eca

	, initialize_igraph = function(directed){
	log_info("Constructing igraph object...")
	# Connected graph
	if (nrow(self$edges) > 0) {
	# A graph with edges
	connectedGraph <- igraph::graph.edgelist(
	as.matrix(self$edges[,list(SOURCE,TARGET)])
	, directed = directed
	)
	} else {
	connectedGraph <- igraph::make_empty_graph(directed = directed)
	}
	# Unconnected graph
	orphanNodes <- base::setdiff(
	self$nodes[, node]
	, unique(c(self$edges[, SOURCE], self$edges[, TARGET]))
	)
	unconnectedGraph <- igraph::make_empty_graph(directed = directed) + igraph::vertex(orphanNodes)
	# Complete graph
	completeGraph <- connectedGraph + unconnectedGraph
	# Store in protected cache
	private$protected$igraph <- completeGraph
	log_info("...done constructing igraph object.")
	return(invisible(NULL))

Here is how to reproduce the exact iGraph object for which we changed expected values in the recent PR (#306 tests/testthat/testdata/node_measures_baseballstats_FunctionReporter.csv). It is an R package build during testing:

git clone https://github.com/uptake/pkgnet.git
cd pkgnet
git reset --hard a904eca16cc8ec7dc5d9429eb1e154fa97934021 # main branch after PR merge

R
install.packages(c("devtools","igraph"))
library(devtools)
library(igraph)

install_local(".") # pkgnet 
library(pkgnet)

install_local("inst/baseballstats")
library(baseballstats)

t <- CreatePackageReport("baseballstats")
t$FunctionReporter$graph_viz # to see the vizual
g <- t$FunctionReporter$pkg_graph # to get the igraph object

ig <- g$igraph
igraph::is_igraph(ig)
# True 

igraph::write_graph(ig, 'baseballstats_igraph.xml', "graphml")

EDIT: Revised above to rely on devtools::install_local()

[szhorvat]

Thank you for the detailed instructions. I am not really an R user, so this makes it much easier for me. However, when following the procedure of the last code block, I'm stuck at

> t <- CreatePackageReport("baseballstats")
INFO [2024-04-09 17:42:53] Creating package report for package baseballstats with reporters: SummaryReporter, DependencyReporter, FunctionReporter
FATAL [2024-04-09 17:42:53] pkgnet could not find an installed package named 'baseballstats'. Please install the package first.
Error in log_fatal(msg) : 
  pkgnet could not find an installed package named 'baseballstats'. Please install the package first.

I tried devtools::install_github('https://github.com/troymoench/baseballstats.git'), but installation still fails. Could you give advice on how I can proceed?

Alternatively, perhaps you could export and upload this specific igraph object, and I can just work with this?

Hopefully there's nothing wrong, but I was surprised to see this PR and I want to make sure that there's no issue in igraph (no incorrect result).

[jayqi]

@szhorvat baseballstats is a dummy package we use for testing that is included in the source of this repo. You can find it here: https://github.com/uptake/pkgnet/tree/main/inst/baseballstats (the relative path should be consistent no matter what commit you're on)

My R is rusty and I'm not sure if this is the best way, but I think devtools::install or devtools::install_local to the local source should work.

[bburns632]

@szhorvat My mistake. I thought referencing an internal script to set up the packages worked, but in reality, I still have some installed in my dev environment from this PR.

I edited the above to leverage devtools as @jayqi mentioned. Should work.

Also, here is the "graphml" file created from that script:
baseballstats_igraph.txt

[szhorvat]

Thanks for the help! I now see what's going on. The change actually happened in 0.10.0, not in 0.10.5 (the latter affecting only undirected graphs, where hub and authority scores are non-interesting anyway).

The bad news:

The network you have is rather unfortunate, as the hub and authority scores are not uniquely defined with it. These scores are the leading eigenvector of $AA^T$ and $A^TA$, respectively. In this case the leading eigenvalue is degenerate, having a two-dimensional eigenspace. $(a, b, b, 0, 0)$ and $(0, a, a, b, 0)$ are a valid solution as hub and authority scores for any $a$ and $b$. For reference,

$$AA^T = \left( \begin{array}{ccccc} 2 & 0 & 0 & 0 & 0 \\ 0 & 1 & 1 & 0 & 0 \\ 0 & 1 & 1 & 0 & 0 \\ 0 & 0 & 0 & 0 & 0 \\ 0 & 0 & 0 & 0 & 0 \\ \end{array} \right)$$

In these cases igraph cannot guarantee a specific result, so future CI failures are possible. The result may in principle depend on things like the BLAS/LAPACK/ARPACK versions used. It's basically out of our control.

The fix that was made in 0.10 was to ensure that the hub and authority scores are returned as a matching pair, as described here:

https://github.com/igraph/igraph/blob/4dcd230d2c0f4ab85f2784d69baa9ccce4e093a5/src/centrality/hub_authority.c#L149-L161

The good news:

• All results were individually correct, both before and after igraph 0.10 (it's just that in earlier versions the hub and authority scores weren't a matching pair).

• I suspect that it is unlikely that igraph will start returning different results for this small graph in practice

[szhorvat]

The tl;dr is that we're (probably) good, no need to do anything for either pkgnet or igraph. A slight change of the network, if possible, would eliminate the degeneracy, but I wouldn't worry about it until you actually start to see inconsistent results.

Thanks for helping me examine this!

pkgnet is a cool package, it was interesting to apply it to igraph itself.

[bburns632]

@szhorvat, thank you so much! If we notice any inconsistent errors, we'll adjust our test packages to avoid this issue altogether.

Thanks for the compliment. We obviously love igraph, and I had to try out a report of igraph as well. Very meta.