sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.38k stars 471 forks source link

Igraph's pagerank not working in Linux #27502

Closed 2460b664-5ecd-43fc-9bbe-d0f333762988 closed 5 years ago

2460b664-5ecd-43fc-9bbe-d0f333762988 commented 5 years ago

Following error arise in linux but works fine in Mac OS.

sage: G = graphs.RandomGNP(1000, .01)
sage: I = G.igraph_graph()
sage: %timeit I.pagerank()

gives following error

        filename = 0x7ffc0bef5337 "/home/rajat/new_version/sage-8.7.beta6/src/bin/sage-ipython"
        module = 0x0
        fp = 0x1b033f0
        p = <optimized out>
        unbuffered = 0
        skipfirstline = 0
        stdin_is_interactive = 1
        help = <optimized out>
        version = <optimized out>
        saw_unbuffered_flag = <optimized out>
        cf = {cf_flags = 0}
#78 0x00007ff538f15830 in __libc_start_main (main=0x4006f0 <main>, argc=3, 
    argv=0x7ffc0bef4708, init=<optimized out>, fini=<optimized out>, 
    rtld_fini=<optimized out>, stack_end=0x7ffc0bef46f8)
    at ../csu/libc-start.c:291
        result = <optimized out>
        unwind_buf = {cancel_jmp_buf = {{jmp_buf = {0, 164082578521821197, 
                4196096, 140720508716800, 0, 0, -162438125174419443, 
                -167616841612554227}, mask_was_saved = 0}}, priv = {pad = {
              0x0, 0x0, 0x0, 0x0}, data = {prev = 0x0, cleanup = 0x0, 
              canceltype = 0}}}
        not_first_call = <optimized out>
#79 0x0000000000400729 in _start ()
No symbol table info available.

Cython backtrace
----------------

29  ../sysdeps/unix/sysv/linux/waitpid.c: No such file or directory.
Traceback (most recent call last):
  File "<string>", line 56, in <module>
  File "/usr/lib/python3/dist-packages/Cython/Debugger/libcython.py", line 689, in invoke
    for arg in string_to_argv(args):
TypeError: argument 1 must be str, not bytes
Saved trace to /home/rajat/.sage/crash_logs/crash_L9MmKW.log
------------------------------------------------------------------------
Unhandled SIGSEGV: A segmentation fault occurred.
This probably occurred because a *compiled* module has a bug
in it and is not properly wrapped with sig_on(), sig_off().
Python will now terminate.
------------------------------------------------------------------------
Segmentation fault (core dumped)

Also see https://groups.google.com/forum/#!topic/sage-devel/NtVP5AQgqNs


updates, tarballs: https://github.com/dimpase/igraph/releases/download/0.7.1999/igraph-0.7.1999.tar.gz and https://github.com/dimpase/python-igraph/releases/download/0.7.1999/python_igraph-0.7.1999.tar.gz

Upstream: Fixed upstream, but not in a stable release.

CC: @dimpase @dcoudert

Component: packages: optional

Author: Dima Pasechnik

Branch/Commit: 721ccc3

Reviewer: David Coudert

Issue created by migration from https://trac.sagemath.org/ticket/27502

sheerluck commented 5 years ago
comment:4

Using latest code from both https://github.com/igraph/igraph.git and https://github.com/igraph/python-igraph.git I get

sage: G = graphs.RandomGNP(1000, .01)
sage: I = G.igraph_graph()
sage: %timeit I.pagerank()
1.27 ms ± 13.4 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)

So upstream have fixed some bugs already

dcoudert commented 5 years ago
comment:5

Thank you for reporting this. So we must update igraph.

@dimpase: I have never tried to add or update an external package. Your help is more than welcome :P

dimpase commented 5 years ago
comment:6

For some reason, upstream closed https://github.com/igraph/igraph/issues/636, providing a workaround (using perzonalized_pagerank() instead), rather than a fix...

dimpase commented 5 years ago

Upstream: Reported upstream. No feedback yet.

dimpase commented 5 years ago
comment:7

I'd like to wait for upstream to react - update packages using git repos rather than upstream tarballs is a bit of a hassle.

dimpase commented 5 years ago
comment:8

OK, upstream says it's fixed, but not in a release, and new releases won't appear any time soon. @rajat1433 - could you make github forks and create new releases there, or I can do this, whatever you prefer.

dimpase commented 5 years ago

Changed upstream from Reported upstream. No feedback yet. to Fixed upstream, but not in a stable release.

2460b664-5ecd-43fc-9bbe-d0f333762988 commented 5 years ago
comment:9

Replying to @dimpase:

OK, upstream says it's fixed, but not in a release, and new releases won't appear any time soon. @rajat1433 - could you make github forks and create new releases there, or I can do this, whatever you prefer.

I don't have much of an experience in creating new releases. If you can guide me through steps I can do it. But since you have an experience you can do it faster so your help is more than welcome.

kiwifb commented 5 years ago
comment:10

We could just cherry-pick https://github.com/igraph/igraph/commit/923974d7520bc49a6b52d34700fb25f0cea66718 taking a snapshot is much more risky and may imply changes of behavior.

dimpase commented 5 years ago
comment:11

I've managed to make a tarball from the master that also passes the testsuite (SAGE_CHECK=yes ./sage -f igraph) it's here https://github.com/dimpase/igraph/releases/tag/0.7.1999

dimpase commented 5 years ago

Commit: 721ccc3

dimpase commented 5 years ago

Branch: u/dimpase/packages/igraph_etc

dimpase commented 5 years ago
comment:12

please review these, and test on OSX. On Linux this passes both SAGE_CHECK=yes ./sage -f igraph python_igraph and the relevant doctests in src/sage/graphs/


New commits:

721ccc3updated igraph and python_igraph to master
dimpase commented 5 years ago

Description changed:

--- 
+++ 
@@ -56,3 +56,10 @@

Also see https://groups.google.com/forum/#!topic/sage-devel/NtVP5AQgqNs + +--- + +updates, tarballs: +https://github.com/dimpase/igraph/releases/download/0.7.1999/igraph-0.7.1999.1.tar.gz +and +https://github.com/dimpase/python-igraph/releases/download/0.7.1999/python_igraph-0.7.1999.tar.gz

dimpase commented 5 years ago

Author: Dima Pasechnik

dcoudert commented 5 years ago
comment:14

What's the right set of instructions to test this patch (I have never done that yet, or I don't remember). Thanks.

dimpase commented 5 years ago
comment:15

You need to download the linked tarballs to upstream/ And the usual git thing, naturally.

dimpase commented 5 years ago
comment:16

And ./sage -f python_igraph

dcoudert commented 5 years ago
comment:17

Thank you.

I add to rename file https://github.com/dimpase/igraph/releases/download/0.7.1999/igraph-0.7.1999.1.tar.gz to https://github.com/dimpase/igraph/releases/download/0.7.1999/igraph-0.7.1999.tar.gz (so .1.tar.gz -> .tar.gz)

Then everything is working well and I can do

sage: G = graphs.PetersenGraph()
sage: I = G.igraph_graph()
sage: I.pagerank()
[0.09999999999999999,
 0.09999999999999999,
 0.10000000000000002,
 0.10000000000000002,
 0.1,
 0.1,
 0.1,
 0.09999999999999999,
 0.1,
 0.09999999999999999]

Someone should double check on linux.

dimpase commented 5 years ago
comment:18

Yep, a typo in file name, sorry. Fixed.

dimpase commented 5 years ago

Description changed:

--- 
+++ 
@@ -60,6 +60,6 @@
 ---

 updates, tarballs:
-https://github.com/dimpase/igraph/releases/download/0.7.1999/igraph-0.7.1999.1.tar.gz
+https://github.com/dimpase/igraph/releases/download/0.7.1999/igraph-0.7.1999.tar.gz
 and
 https://github.com/dimpase/python-igraph/releases/download/0.7.1999/python_igraph-0.7.1999.tar.gz
dcoudert commented 5 years ago
comment:19

I'm no able to access https://github.com/dimpase/igraph/releases/download/0.7.1999/igraph-0.7.1999.tar.gz

dimpase commented 5 years ago
comment:20

Sorry, my fault. Please try now (and check that it is identical to the one you renamed).

dcoudert commented 5 years ago
comment:21

For me the files are the same.

I tried on a linux computer and it's working well. I can use the pagerank method of igraph.

For me this patch is good to go. Should we update the ticket title and description before setting it to positive review?

dimpase commented 5 years ago
comment:22

How about we add a doctest with pagenumber computation?

kiwifb commented 5 years ago
comment:23

Should we really? We had those conversations in the last few weeks. Do we really want to add tests to sage about bugs in upstream packages? No, if anything the test belongs to the upstream package. This is a case in point and adding a test to sage is scope creep.

dcoudert commented 5 years ago
comment:24

We plan to add method pagerank to generic graphs. It will call either methods of networkx (pure Python, or using numpy or scipy), or if installed igraph. So we will certainly have a test to check that it's working well, as we have with other optional packages (e.g., bliss, tdlib, etc.).

kiwifb commented 5 years ago
comment:25

That is fine, you are just checking that sage works, not that an upstream package as been fixed through sage.

jhpalmieri commented 5 years ago
comment:26

For what it's worth, with this branch I get two failures when I do sage -f -c igraph on OS X Mojave:

...
124: Merging layouts 2 (igraph_i_layout_merge):      FAILED (layout.at:62)
...
241: SCG of a graph, stochastic matrix (igraph_scg) : FAILED (scg.at:73)
...
ERROR: 249 tests were run,
2 failed unexpectedly.
2 tests were skipped.
## -------------------------- ##
## testsuite.log was created. ##
## -------------------------- ##

The previous version of igraph failed two other (?) tests, so I don't think the new failures should necessarily be an obstacle for a positive review. Failures from old version:

...
 25: Sparse matrix, solvers (igraph_sparsemat_t):    FAILED (types.at:148)
...
 32: Another sparse matrix (igraph_spmatrix_t):      FAILED (types.at:183)
...
ERROR: All 234 tests were run,
2 failed unexpectedly.
## -------------------------- ##
## testsuite.log was created. ##
## -------------------------- ##
dcoudert commented 5 years ago

Reviewer: David Coudert

dcoudert commented 5 years ago
comment:27

I also tried sage -f -c igraph with

I set this ticket as positive review. It fixes some issues, and I don't know where pre-existing issues come from.

embray commented 5 years ago
comment:28

Ticket retargeted after milestone closed (if you don't believe this ticket is appropriate for the Sage 8.8 release please retarget manually)

vbraun commented 5 years ago

Changed branch from u/dimpase/packages/igraph_etc to 721ccc3