Enable simplification of multi-relation edges & make multi-network construction work with igraph version 2.0.1.1

[bockthom]

Prerequisites

• I adhere to the coding conventions (described here) in my code.
• I have updated the copyright headers of the files I have modified.
• I have written appropriate commit messages, i.e., I have recorded the goal, the need, the needed changes, and the location of my code modifications for each commit. This includes also, e.g., referencing to relevant issues.
• I have put signed-off tags in all commits.
• I have updated the changelog file NEWS.md appropriately.
• I have checked whether I need to adjust the showcase file showcase.R with respect to my changes.
• The pull request is opened against the branch dev.

Description

This PR addresses multiple issues:

• It fixes Error message logging in metrics.smallwordness is broken #246 (i.e., fixes a broken logerror statement)
• If adjusts multi-network construction such that it works with the recently released igraph version 2.0.1.1, in which it is not possible any more to add an empty list of vertices. A simple check for an empty list fixes this problem.
• It updates the README regarding the effectiveness of network conf parameter "artifact.directed", which now is effective as of PR Introduce issue-based artifact-network creation, new tests and fix current tests + minor bug fixes #244.
• It enables multi-relation edges to be simplified. That is, the simplify methods get an additional parameter and also the network configuration gets an additional parameter to decided whether edges between a pair of vertices should be contracted to a single edge when the edges are of different relations. The default behavior stays as before, that is, edges of different relations are not contracted during simplification. However, notice that the implementation of this feature is not complete, as also the edge attribute handling needs to be adjusted accordingly, and also proper tests are missing. I leave this open to a future PR - I have summarized the steps that are still missing in issue Fix edge-attribute handling when simpliyfing networks #251.

Changelog

Added

• Add the possibility to simplify edges of multiple-relation networks into a single edge at all instead of a single edge per relation (PR Enable simplification of multi-relation edges & make multi-network construction work with igraph version 2.0.1.1 #250, 2105ea8)

Changed/Improved

• Explicitly add R version 4.3 to the CI test pipeline (9f346d5)

Fixed

• Fix broken error logging in metrics.smallworldness (03e0688)
• Fix multi-network construction to work with igraph version 2.0.1.1, which does not allow to add an empty list of vertices (PR Enable simplification of multi-relation edges & make multi-network construction work with igraph version 2.0.1.1 #250, 5547896)

[szhorvat]

• the recently released igraph version 2.0.1.1, in which it is not possible any more to add an empty list of vertices

Can you please elaborate on this, and give an example of what changed? This may not be an intentional change.

[bockthom]

Can you please elaborate on this, and give an example of what changed? This may not be an intentional change.

Thanks for having a look at our pull request @szhorvat 😄 Sure, I will elaborate on this:

Up until now, we used the "+" operator and igraph::vertices(vertices.to.add, type = type.of.the.vertices.to.add) to add some vertices to a given graph later on. In this situation, vertices.to.add is a vector that could potentially be empty in cases where we don't want to add any vertex, as vertices.to.add is the outcome of some other computations. In particular, if we don't want to add any vertex, it happens that vertices.to.add = character(0), which now leads to the following error in the following example:

graph + igraph::vertices(vertices.to.add, type = type.of.the.vertices.to.add)

Error: Error in `add_vertices(e1, la, attr = e2)`: At rinterface_extra.c:78 : Expecting a scalar integer but received a vector of length 2. Invalid value

As I assumed that character(0) may not be allowed any more in such cases (you might have some good reasons for that), I just fixed this on our side by adding an additional check for length(vertices.to.add) > 0 before executing the statement. If the change in behavior was not intended, I hope my description is helpful for you.

[bockthom]

@szhorvat I just encountered another problem in our failing tests which is also related to + igraph::vertices:

author.net =
    igraph::make_empty_graph(n = 0, directed = FALSE) +
    igraph::vertices("A", "B", "C", type = "author", kind = "author") +
    igraph::edges("A", "A", type = "intra", relation = "mail")

In previous versions of igraph, this did not fail. However, with the current igraph version, this fails:
Error in add_vertices(e1, la, attr = e2) : At rinterface_extra.c:78 : Expecting a scalar integer but received a vector of length 2. Invalid value

What I figured out now is the following: Previously, the attributes have automatically been expanded. That is, if I add 3 vertices but the type attribute contains only one element, then the type-attribute vector was automatically expanded and added as an attribute to each added vertex. Now, the code is failing if the attribute vector is not equal to the number of added vertices. (This might also be the problem in the example that I stated some hours ago, where the attribute vector had length one but zero vertices should be added).
I am not sure if this is an intended change in igraph, but the previous version was much more comfortable in this regard - if you want to add numerous vertices that should all have the same value for an attribute, then it was fine to just add an attribute vector of length one and having this one value automatically applied to all vertices... Are you going to fix this again or do we need to fix this on our side by expanding the vector length properly?

[szhorvat]

So it seems that the problem is that if the number of attribute values provided does not match the number of vertices or edges added, then there's an error, even if a single attribute value was given.

This works:

g<-make_empty_graph(1) + vertices('a', 'b', foo=c(2,3))

This does not work, and rightfully so:

g<-make_empty_graph(1) + vertices('a', 'b', foo=c(2,3,4))

But one might expect this to assign na attribute value of 5 to both vertices, yet it does not work:

g<-make_empty_graph(1) + vertices('a', 'b', foo=5)

Am I understanding you correctly?

Pinging @krlmlr to ask if this is indeed expected.

[bockthom]

So it seems that the problem is that if the number of attribute values provided does not match the number of vertices or edges added, then there's an error, even if a single attribute value was given.

Correct.

This works:

g<-make_empty_graph(1) + vertices('a', 'b', foo=c(2,3))

Sure, this should work.

This does not work, and rightfully so:

g<-make_empty_graph(1) + vertices('a', 'b', foo=c(2,3,4))

Agreed, this should not work.

But one might expect this to assign an attribute value of 5 to both vertices, yet it does not work:

g<-make_empty_graph(1) + vertices('a', 'b', foo=5)

Am I understanding you correctly?

Yes, I totally agree with your understanding.

Pinging @krlmlr to ask if this is indeed expected.

I am not sure if the behavior with regard to multiple vertices and a single-element attribute vector was expected in past versions of igraph - but it was a nice convenience feature to add multiple vertices at once that should all have the same vertex attributes - then passing just one value for each attribute is much more readable and faster to write than extending all the vectors for all attributes to the number of vertices to add. We used this feature of igraph in our test suite for more than 6 years already. But if you think that this should be changed, it is easy to fix on our side. Nevertheless, I'd also be glad if you can restore the previous behavior in igraph 😉

[krlmlr]

With igraph 1.6.0:

options(conflicts.policy = list(warn = FALSE))
library(igraph)

g <-
  make_empty_graph(1) +
  vertices("a", "b", foo = 5)

V(g)$foo
#> [1] NA  5  5

^{Created on 2024-02-09 with reprex v2.1.0}

We clearly had no tests for this, and I agree that this is idiomatic to R.

[hechtlC]

Looks good thank you @bockthom
I will merge this now