chore: igraph 0.10

[Antonov548]

• Remove scg related functions
• FIx stimulus to add reference (&) to input arguments (related to missed types in types-RC.yaml)
• Use int for igraph_bool_t - changed in vendor/cigraph
• Fix stimulus to check return type of function
• Remove multi in get.edge.ids file interface.R
• Change R interface for R_igraph_weighted_adjacency - weights is not string anymore
• Configure config.h.in

Closes #753.

[krlmlr]

Thanks. How do I build or install the package now?

[Antonov548]

Thanks. How do I build or install the package now?

I was writing you email.

I have removed 0.9 files. There is now build_install.sh script. And after this you can try R CMD INSTALL

[krlmlr]

Thanks. We still need all sources under src/ or perhaps elsewhere so that we can use devtools::install_github() . I placed them in src/vendor/cigraph for now, that would be the idiomatic place. But we could also have it in ./cigraph, just not as a submodule (we discussed that before).

I'm now getting errors from CMake when running sh build_install.sh .

Can you work with that? Please revert if needed.

Eventually, ./configure should contain what build_install.sh contains now, and only that.

[szhorvat]

There is now build_install.sh script. And after this you can try R CMD INSTALL

Just so I know what to expect, is this supposed to work at the moment? (It doesn't here.)

Some things that appear to be missing:

• USING_R must be defined when compiling the C core
• The igraph_bool_t type must be adjusted to be an int.

[Antonov548]

Can you work with that? Please revert if needed.

I have moved vendor which related to cigraph to cigraph folder, since it's required to build igraph library

[Antonov548]

• USING_R must be defined when compiling the C core

I have added USING_R definition. It is also require R headers. I have used cmake script which we used before to find R headers.

[Antonov548]

@szhorvat Seems there is now some missed types for stimulus in types-RC.yaml. Should we add all new types as I did in last commit?

[krlmlr]

The -O harms cmake --build . The following works:

diff --git a/configure b/configure
index 9364713f7..9c75ea263 100755
--- a/configure
+++ b/configure
@@ -13,7 +13,7 @@ fi
 # Build igraph with cmake
 cmake -S . -B build -DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=install

-cmake --build build --config Release
-cmake --install build --config Release
+MAKEFLAGS="-j 8" cmake --build build --config Release
+MAKEFLAGS="-j 8 -O" cmake --install build --config Release

 cp -r ./install/include/igraph ./include

[ntamas]

You're right, just set it to ON for now, and we'll need to figure out how to link to R's version at a later point.

I think that CMake looks at certain default, system-wide directories when it's looking for libraries, but you can prepend your own directories to this list with some CMake setting (I don't remember off the top of my head and I'm not at my desk now). The easiest would probably be to prepend R's preferred library and include folders to CMake's list and let the auto-detection do its job.

[szhorvat]

That's a bit complicated due to the non-standard library names used by R. The following works on my system though, and could be used as a starting point for a proper solution.

LDFLAGS=-L/Library/Frameworks/R.framework/Libraries/ cmake .. -DBLAS_LIBRARIES=libRblas.dylib -DLAPACK_LIBRARIES=libRlapack.dylib -DBLA_VENDOR=Generic -DIGRAPH_USE_INTERNAL_BLAS=OFF -DIGRAPH_USE_INTERNAL_LAPACK=OFF

• -DBLAS_LIBRARIES=libRblas.dylib -DLAPACK_LIBRARIES=libRlapack.dylib These set the library names. I'm not sure if an absolute path is better / more robust than using LDFLAGS with simple names.
• -DBLA_VENDOR=Generic May not be needed, attempt to prevent the detection of other BLAS implementations that may be present on the system
• -DIGRAPH_USE_INTERNAL_BLAS=OFF -DIGRAPH_USE_INTERNAL_LAPACK=OFF These are to ensure that we get an error if something goes wrong, and CMake won't just pick up an alternative BLAS.

[szhorvat]

I can't get why build is failing on CI. There is no includes which should be provided from Makevars? Does CI use some another Makevars?

I can reproduce that failure if I remove the system-wide install of igraph I had.

The code that caused the failure seems completely unnecessary. I'm removing it in the below commit.

EDIT: CI won't run until the conflict with the base branch is resolved.

[ntamas]

Maybe we should consider using https://github.com/xtensor-stack/xtensor-r/blob/master/cmake/FindR.cmake to find R, and then we would get the full path to R's BLAS and LAPACK libraries for free?

[krlmlr]

I minimized the diff with the main branch so that there is a tiny chance that we can actually review what's happening here.

Is this PR doing too much? Should we keep the data format unchanged for now, continue operating with double vectors in the R interface, and use 64-bit integers in the C core? It seems that we're copying data anyway -- we could copy and raise an error in case of out-of-bounds conditions.

[Antonov548]

library(igraph)

g <- make_ring(4)
print(g)

It works now

[Antonov548]

@szhorvat I found a reason of some failing tests. I believe a lot of tests are failing to this reason. How we should deal with cache property in igraph_t? Should be also copied to R structure?

The problem is that we don't copy cache -> then read graph -> don't read cache -> try delete cache, but it's empty pointer.

[ntamas]

Hmmm yes, the cache is a new thing in igraph 0.10; its purpose is to store certain boolean flags about the graph if we already know what their value is. For instance, if we know that the graph has no loop edges (because we have determined it earlier), we can store it as a flag and subsequent calls to the same graph can re-use the cached value without having to check for loop edges again. Essentially the cache is a layer of two boolean vectors: one that tells us for each potentially cacheable boolean feature whether it is currently cached or not, and the other vector tells us what the cached value is (undefined if the property is not cached).

Now, we can do it in C because we know that all manipulations to the graph structure go through our own functions where we manage and maintain the cache. In R, it's not that clear as one can easily see the internals of the graph with a call to unclass() and then mess around with the internal vectors without updating the cache. On the other hand, we can argue that the internals of the graph are private, and anyone who calls unclass() on a graph has committed a primordial sin. If we can say that, then we can safely copy the cache to the R side and back.

[Antonov548]

primordial sin

Thanks for the explanation. I will try to copy cache.
Should I also prepare pull request with SINS.md? :)

[krlmlr]

The cache could also be an external pointer. It will be lost during serialize(), but then it's a cache.

Before we spend too much time here, I'd like to investigate storing the graph object as a raw vector. This can be done in the main branch, independent of this branch. I'll open a new issue.

[ntamas]

Circling back to the detection of BLAS: does the standardized R compilation environment (the one used on CRAN) contain pkg-config and is it configured to find R's BLAS / LAPACK first? If so, we could also use CMake's BLA_PREFER_PKGCONFIG input variable of the FindBLAS.cmake to ask CMake to prefer the BLAS instance returned by pkg-config.