Opened 3 years ago

Closed 3 years ago

#19483 closed defect (fixed)

Update bliss

Reported by: ncohen Owned by:
Priority: major Milestone: sage-6.10
Component: graph theory Keywords:
Cc: azi Merged in:
Authors: Nathann Cohen Reviewers: Jernej Azarija, Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: ea80cfd (Commits) Commit: ea80cfd6f3e3e9f61d8d64a560d22428a9509dd7
Dependencies: Stopgaps:

Description (last modified by ncohen)

This update also fixes the following problem:

sage: DiGraph('GY@OQ?OW@?O?').canonical_label()
<crash>

new tarball: http://www.steinertriples.fr/ncohen/tmp/bliss-0.73.tar.gz

Change History (17)

comment:1 Changed 3 years ago by azi

Yes this is a bug in bliss (confirmed by the author) that was fixed in the recent version!

So the solution is simply to update the spkg to use the latest version from here http://www.tcs.hut.fi/Software/bliss/

Last edited 3 years ago by azi (previous) (diff)

comment:2 Changed 3 years ago by ncohen

  • Authors set to Nathann Cohen
  • Branch set to u/ncohen/19483
  • Description modified (diff)
  • Summary changed from Crash with bliss when tesing posets.py to Update bliss

Hello Jernej,

I updated the package but the bug remains. Could you look into it?

Nathann

comment:3 Changed 3 years ago by git

  • Commit set to 3152b5c3e970c906f48c9bf1cf001aee4c2583f6

Branch pushed to git repo; I updated commit sha1. New commits:

comment:4 Changed 3 years ago by git

  • Commit changed from 3152b5c3e970c906f48c9bf1cf001aee4c2583f6 to 579aed862ea02a060a82e6b03ab7cd068a15f76e

Branch pushed to git repo; I updated commit sha1. New commits:

579aed8trac #19483: Update bliss

comment:5 Changed 3 years ago by azi

Hey there!

That is weird. As far as bliss is concerned I can reproduce the bug in bliss-0.72 but not in bliss-0.73

azi@bodysnatcher:~/bliss-0.72$ ./bliss -directed -can -sh=f /tmp/foo.dimacs 
bliss: graph.cc:3875: virtual bool bliss::Digraph::nucr_find_first_component(unsigned int, std::vector<unsigned int>&, unsigned int&, bliss::Partition::Cell*&): Assertion `sh_return' failed.
Aborted (core dumped)

And

azi@bodysnatcher:~/bliss-0.73$ ./bliss -directed -can -sh=f /tmp/foo.dimacs 
Generator: (2,3)(6,7)
Generator: (3,5)(4,6)
Canonical labeling: (1,8)(2,7)(3,6)
Nodes:          6
Leaf nodes:     4
Bad nodes:      0
Canrep updates: 1
Generators:     2
Max level:      2
|Aut|:          6
Total time:	0.00 seconds

Every other heuristic -sh works as well in bliss-0.73. So that makes me wonder, are we 100% that the spkg really installs bliss?

Last edited 3 years ago by azi (previous) (diff)

comment:6 follow-up: Changed 3 years ago by ncohen

  • Description modified (diff)
  • Status changed from new to needs_review

Mystery cleared: that's because installing the new bliss is not enough--> one also has to touch bliss.pyx, as otherwise the code is not recompiled.

Nathann

comment:7 Changed 3 years ago by azi

  • Reviewers set to Jernej Azarija
  • Status changed from needs_review to positive_review

comment:8 Changed 3 years ago by ncohen

Thanks,

Nathann

comment:9 in reply to: ↑ 6 Changed 3 years ago by jdemeyer

  • Status changed from positive_review to needs_work

Replying to ncohen:

Mystery cleared: that's because installing the new bliss is not enough--> one also has to touch bliss.pyx, as otherwise the code is not recompiled.

That's a bug.

comment:10 Changed 3 years ago by jdemeyer

Use cp instead of mv in spkg-install (this is probably why you have the dependency issue).

comment:11 Changed 3 years ago by jdemeyer

Note that you still need an additional run of make or ./sage -b after installing bliss. However, that's not the fault of the package.

comment:12 Changed 3 years ago by git

  • Commit changed from 579aed862ea02a060a82e6b03ab7cd068a15f76e to ea80cfd6f3e3e9f61d8d64a560d22428a9509dd7

Branch pushed to git repo; I updated commit sha1. New commits:

ea80cfdtrac #19483: Bugfix

comment:13 follow-up: Changed 3 years ago by ncohen

Sorry sorry, I am a bit sick these days and with half of my brain I did not realise that indeed this 'touch' thing was a problem. I added an empty line in the file.

I did not understand what you said about the mv/cp change but I did it anyway. Could you be more verbose about it? I do not know of which dependency problem you are talking.

Thanks, and sorry if the questions are stupid. Just slept 12 hours, and I may have to do the same tonight :-/

Nathann

comment:14 Changed 3 years ago by ncohen

  • Status changed from needs_work to needs_review

comment:15 in reply to: ↑ 13 Changed 3 years ago by jdemeyer

  • Reviewers changed from Jernej Azarija to Jernej Azarija, Jeroen Demeyer
  • Status changed from needs_review to positive_review

Replying to ncohen:

I did not understand what you said about the mv/cp change but I did it anyway. Could you be more verbose about it? I do not know of which dependency problem you are talking.

Sorry, it wasn't clear indeed.

The "dependency problem" is the problem that you manually needed to touch the .pyx file. Let me explain more:

  • with mv, the file is moved with the same timestamp, which in practice is the timestamp the package was made. So, Cython thinks this is an old file and sees no reason to recompile bliss.pyx.
  • with cp, the new file really is a new file so it has a new timestamp. Cython will detect this file as changed and automatically recompile bliss.pyx.

comment:16 Changed 3 years ago by ncohen

Oh, I see. Cool. Then I added this empty line uselessly. It's a good news though, it felt weird that Cython would not detect that :-)

Thanks,

Nathann

comment:17 Changed 3 years ago by vbraun

  • Branch changed from u/ncohen/19483 to ea80cfd6f3e3e9f61d8d64a560d22428a9509dd7
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.