# The unit tests thread

**URL:** <https://igraph.discourse.group/t/the-unit-tests-thread/597>\
**Category:** Development\
**Tags:** C\
**Created:** [21 January 2021 11:22 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597 "2021-01-21T11:22:21Z")\
**Posts on this page:** 17\
**Page:** 3

<div class="post-metadata">

**Author:** ![szhorvat](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/szhorvat/32/3_2.png) [@szhorvat](https://igraph.discourse.group/u/szhorvat)\
**Post date:** [2 March 2021 15:58 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/42 "2021-03-02T15:58:56Z")

</div>

@GroteGnoom I saw your question about issue priorities before you deleted it. The issues are not very well prioritized at the moment. It would take some effort to triage them. There are priority labels, but they are not consistently applied at the moment.

If you’d like to work on some issues, I’d suggest to choose based on what you prefer and what you are comfortable with. If you give us some specifics about what you’re looking for (bug fix, new feature, infrastructure work, etc.) we can make some more concrete suggestions.

---

<div class="post-metadata">

**Author:** ![GroteGnoom](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/grotegnoom/32/428_2.png) [@GroteGnoom](https://igraph.discourse.group/u/GroteGnoom)\
**Post date:** [4 March 2021 11:01 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/43 "2021-03-04T11:01:22Z")

</div>

In general when selecting issues I don’t have very specific preferences, and think I can handle a few mathematical challenges, although it might take me a while if I’m not familiar with the subject. I also don’t mind reading papers if that’s necessary. It’s nice for me to always have something to work on, and I prefer writing new C code to testing. Fixing bugs is also interesting. The reason I started testing is because there’s such a clear list of things to work on without really asking where I should start. I can just ‘assign’ myself, and I always have at least some work to do.  
I can handle some infrastructure work as well, but I have to learn a bit more about CMake and the other tools we’re using.  
I think the nicest projects are ones where I can work on for days without bothering anyone.

---

<div class="post-metadata">

**Author:** ![szhorvat](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/szhorvat/32/3_2.png) [@szhorvat](https://igraph.discourse.group/u/szhorvat)\
**Post date:** [4 March 2021 14:55 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/44 "2021-03-04T14:55:45Z")

</div>

An easy project that would contribute a genuinely useful new feature is this:

> <https://github.com/igraph/igraph/issues/1223>
>
> Implement an algorithm to compute the pseudo-diameter of a graph. This is a means of obtaining a quick approximation (and lower...

---

<div class="post-metadata">

**Author:** ![GroteGnoom](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/grotegnoom/32/428_2.png) [@GroteGnoom](https://igraph.discourse.group/u/GroteGnoom)\
**Post date:** [4 March 2021 14:59 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/45 "2021-03-04T14:59:26Z")

</div>

Allright, looks good 🙂 Can you assign me?

---

<div class="post-metadata">

**Author:** ![szhorvat](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/szhorvat/32/3_2.png) [@szhorvat](https://igraph.discourse.group/u/szhorvat)\
**Post date:** [4 March 2021 15:01 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/46 "2021-03-04T15:01:54Z")

</div>

Some more projects that I am personally interested in, but haven’t looked into their difficulty yet:

> <https://github.com/igraph/igraph/issues/1397>
>
> I would like to see "K shortest paths" functionality added to the core of igraph. This function would find the K...

> <https://github.com/igraph/igraph/issues/1324>
>
> Vizing's theorem states that any simple graph's edges can be coloured with either Delta or Delta + 1 colours, where Delta...

> <https://github.com/igraph/igraph/issues/1175>
>
> In the long term, it would be nice to extend igraph\_lattice so it can generate a lattice that is periodic along...

> <https://github.com/igraph/igraph/issues/1452>
>
> The Fulkerson-Ryser test, used in is\_graphical for simple directed graphs, can (and should) be checked in linear time (assuming that the...

> <https://github.com/igraph/igraph/issues/932>
>
> A function to compute the transitive reduction of a directed graph would be nice.
> a transitive reduction of a directed graph is...

Generally, feel free to choose any issue (even if it is assigned to someone), just ask us before you start. I won’t be able to work on most issues that are assigned to me in the near future, but I might have some ideas about how to do them.

---

<div class="post-metadata">

**Author:** ![szhorvat](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/szhorvat/32/3_2.png) [@szhorvat](https://igraph.discourse.group/u/szhorvat)\
**Post date:** [8 March 2021 14:19 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/47 "2021-03-08T14:19:20Z")

</div>

There are now experimental live test coverage reports: [Codecov](https://app.codecov.io/gh/igraph/igraph)

---

<div class="post-metadata">

**Author:** ![GroteGnoom](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/grotegnoom/32/428_2.png) [@GroteGnoom](https://igraph.discourse.group/u/GroteGnoom)\
**Post date:** [9 March 2021 15:10 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/48 "2021-03-09T15:10:43Z")

</div>

I am testing igraph\_laplacian\_spectral\_embedding.  
I have notices a few issues:

1. It’s supposed to give U, D and V such that L=U D V^T, but it doesn’t for my test cases. L=U D V^(-1) does seem to hold.
2. For directed graphs it expects type == IGRAPH\_EMBEDDING\_OAP. This type is not mentioned in the documentation.
3. For a directed graph with one edge between two vertices, the function fails:

```auto
Two_vertices, one edge:
Error at src/linalg/arpack.c:539 : ARPACK error, 2x2 matrix is not symmetric - Invalid value.
Error at ../../src/misc/embedding.c:711 : - Invalid value.
Error at ../../src/misc/embedding.c:984 : - Invalid value.
Fatal error at igraph_laplacian_spectral_embedding.c:30 : Assertion failed: igraph_laplacian_spectral_embedding( g, no_eigenv, weights, IGRAPH_EIGEN_LM, IGRAPH_EMBEDDING_OAP, 0, &X, &Y, &D, options) == IGRAPH_SUCCESS

```

In the adjacency\_spectral\_embedding unit test a directed graph is used, so it shouldn’t be a general problem of not being able to handle asymmetric matrices.

Now my question is: Should I spend a lot of time fixing these issues? Or should I leave it up to someone with more knowledge of the intended functionality of this function? For issue 1. for example I don’t know if the documentation is wrong or the implementation is wrong, and it will take a lot of reading to figure that out.

---

<div class="post-metadata">

**Author:** ![szhorvat](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/szhorvat/32/3_2.png) [@szhorvat](https://igraph.discourse.group/u/szhorvat)\
**Post date:** [9 March 2021 18:27 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/49 "2021-03-09T18:27:36Z")

</div>

I’ve never used this function myself. @Gabor you implemented it originally. Do you have a quick answer to the above question?

---

<div class="post-metadata">

**Author:** ![GroteGnoom](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/grotegnoom/32/428_2.png) [@GroteGnoom](https://igraph.discourse.group/u/GroteGnoom)\
**Post date:** [9 April 2021 10:48 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/50 "2021-04-09T10:48:57Z")

</div>

igraph\_neighbors says it returns ‘Adjacent vertices to a vertex’, but when there is a loop it will list the starting vertex itself, and when there are multiple edges to another vertex it will list the other vertex multiple times.

igraph\_dyad\_census expects unique non-loop neighbors to be returned, and when working on pseudo-diameters I also wanted a function that did this. dyad\_census now returns incorrect results when loops or multiple edges are present.

One way to work around this is to use adjlists, because there you can indicate NO\_LOOPS, and NO\_MULTIPLE.

Should igraph\_neigbors also take parameters that indicate how loops and multiple edges should be handled? Should its results be changed so it doesn’t use loops and multiple edges? Those would take quite a bit of work because every call needs to be checked. But maybe that needs to be done anyway to see if multigraphs are handled correctly.  
We could also add a new function which takes more parameters.  
I could also just continue to handle each case separately, and change the docs to igraph\_neighbors to warn for this issue.

---

<div class="post-metadata">

**Author:** ![szhorvat](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/szhorvat/32/3_2.png) [@szhorvat](https://igraph.discourse.group/u/szhorvat)\
**Post date:** [9 April 2021 11:05 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/51 "2021-04-09T11:05:30Z")

</div>

> [@GroteGnoom](#):
>
> Should igraph\_neigbors also take parameters that indicate how loops and multiple edges should be handled?

There’s been a debate about this, but my personal take is a strong _YES, it should_.

Or at least there should be an internal variant of the function which provides this (but then why not make it public?)

> [@GroteGnoom](#):
>
> Should its results be changed so it doesn’t use loops and multiple edges?

And that’s a strong no from me. Subjectively, most uses cases require the current behaviour. Most other graph packages which support non-simple graph have the same behaviour. The current behaviour is in many (but not all…) ways consistent with degrees, adjacency matrices, etc.

---

<div class="post-metadata">

**Author:** ![GroteGnoom](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/grotegnoom/32/428_2.png) [@GroteGnoom](https://igraph.discourse.group/u/GroteGnoom)\
**Post date:** [9 April 2021 11:57 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/52 "2021-04-09T11:57:55Z")

</div>

I should have searched first: [neighbors() and incident() should allow controlling loop inclusion · Issue #1737 · igraph/igraph · GitHub](https://github.com/igraph/igraph/issues/1737). I’ll continue from there.

---

<div class="post-metadata">

**Author:** ![GroteGnoom](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/grotegnoom/32/428_2.png) [@GroteGnoom](https://igraph.discourse.group/u/GroteGnoom)\
**Post date:** [23 April 2021 09:38 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/53 "2021-04-23T09:38:31Z")

</div>

Quite a big part of code that isn’t covered according to Codecov are external libraries like Bliss and Gengraph. Should we treat them the same way as home-built igraph code?

Reasons to test: We have no reason to assume they’re of perfect quality.

Reasons not to test: Maybe we’ll be doing double work if they’re already tested in other ways, or there are parts in it we don’t need. Maybe we’ll use them as external packages in the future and it somehow seems weird to test external packages, even if the situation isn’t much different.

---

<div class="post-metadata">

**Author:** ![szhorvat](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/szhorvat/32/3_2.png) [@szhorvat](https://igraph.discourse.group/u/szhorvat)\
**Post date:** [23 April 2021 15:25 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/54 "2021-04-23T15:25:20Z")

</div>

The coverage report is just a tool, not an end in itself. There is no reason to test code that we don’t use, so don’t spend time on it.

The question is rather: why not remove the unused parts of these included libraries? Well, that kind of surgery also takes care, time, and effort. Some parts might be used in the future. Some libraries (gengraph) are really not maintained outside of igraph, not well documented, and are just easier to understand as a whole, so I’m reluctant to strip out code just to increase the coverage numbers. The extra code has very few drawbacks: while it increases the size of the _static_ library slightly, it does not impact the shared library or executables linked to igraph.

---

<div class="post-metadata">

**Author:** ![GroteGnoom](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/grotegnoom/32/428_2.png) [@GroteGnoom](https://igraph.discourse.group/u/GroteGnoom)\
**Post date:** [26 April 2021 08:44 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/55 "2021-04-26T08:44:56Z")

</div>

> [@szhorvat](#):
>
> The coverage report is just a tool, not an end in itself. There is no reason to test code that we don’t use, so don’t spend time on it.

Yes, we shouldn’t just make some number higher if that doesn’t contribute to our code quality. But one advantage of the tool is that we can see what work needs to be done. Right now, that isn’t really clear. Shall we add the external libraries to the ignore paths?

---

<div class="post-metadata">

**Author:** ![szhorvat](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/szhorvat/32/3_2.png) [@szhorvat](https://igraph.discourse.group/u/szhorvat)\
**Post date:** [26 April 2021 12:18 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/56 "2021-04-26T12:18:36Z")

</div>

Maybe only certain parts of the external libraries …

In some cases, it was useful to see which parts of their functionality was tested. For exampl, with PRPACK, it helped me notice that it switches methods depending on the size of the graph. One of those branches was not tested, and indeed it turned out to be buggy.

We should not test external libraries _directly_. But we should test the igraph functions that use them, and fix any issues that affect those functions. With the PRPACK example, this meant computing PageRank both for graphs smaller and larger than 128 vertices.

---

<div class="post-metadata">

**Author:** ![szhorvat](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/szhorvat/32/3_2.png) [@szhorvat](https://igraph.discourse.group/u/szhorvat)\
**Post date:** [26 April 2021 12:33 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/57 "2021-04-26T12:33:49Z")

</div>

@GroteGnoom Looking at the bliss coverage, I realized that not all splitting heuristics were exercised. This is a first attempt to exercise them better: [tests: exercise all splitting heursitics in bliss · igraph/igraph@9836767 · GitHub](https://github.com/igraph/igraph/commit/983676712cd40005024d69b19f90dd4e4c414dd3) The coverage increased by a whopping 0.5%, but there are also failures.

This experience tells me that we shouldn’t exclude external libraries.

---

<div class="post-metadata">

**Author:** ![szhorvat](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/szhorvat/32/3_2.png) [@szhorvat](https://igraph.discourse.group/u/szhorvat)\
**Post date:** [26 April 2021 13:05 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/58 "2021-04-26T13:05:45Z")

</div>

This test failure was my fault: I remove the RNG seeding because I did not realize that `vector_shuffle` made it necessary. It’s fixed now.

[Previous page](https://igraph.discourse.group/t/the-unit-tests-thread/597.md?page=2)
