Opened 5 years ago

Closed 5 years ago

#19919 closed enhancement (fixed)

upgrade nauty to version 26 and make it standard

Reported by: dimpase Owned by:
Priority: major Milestone: sage-7.1
Component: packages: standard Keywords:
Cc: jmantysalo, ncohen, jdemeyer Merged in:
Authors: Dima Pasechnik Reviewers: François Bissey, Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: 56815d4 (Commits) Commit: 56815d4da72ff750813797d9158a82b1de9cd199
Dependencies: Stopgaps:

Description (last modified by dimpase)

We can now make nauty standard; tarball can be obtained from http://users.ox.ac.uk/~coml0531/sage/nauty-26r1.tar.gz

(this is just renamed upstream tarball, inserted '-' in the name)

Change History (44)

comment:1 Changed 5 years ago by dimpase

  • Description modified (diff)
  • Summary changed from upgrade nasty to version 26 and make it standard to upgrade nauty to version 26 and make it standard

comment:2 Changed 5 years ago by dimpase

  • Cc jmantysalo ncohen jdemeyer added
  • Status changed from new to needs_review

comment:3 follow-up: Changed 5 years ago by vbraun

This branch doesn't make it standard, we should have a formal vote for that as well I guess.

A spkg-check script that runs runalltests would be nice.

comment:4 Changed 5 years ago by git

  • Commit changed from 3a2555bd253634c9cee57a146ae81c1d703ff829 to 076a8b5ceb7df52d62bccc09ed71a3bb099e248d

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

076a8b5package type and source repo

comment:5 in reply to: ↑ 3 Changed 5 years ago by dimpase

Replying to vbraun:

This branch doesn't make it standard, we should have a formal vote for that as well I guess.

fixed and called up for vote...

A spkg-check script that runs runalltests would be nice.

but that's what spkg-check already does, no?

#!/usr/bin/env bash

if [ "$SAGE_LOCAL" = "" ]; then
   echo >&2 "SAGE_LOCAL undefined ... exiting";
   echo >&2 "Maybe run 'sage -sh'?"
   exit 1
fi

cd nauty*

# runalltests doesn't exit with a zero status.
# So we check $fails instead.

$MAKE checks
source ./runalltests

if [ "${fails}" -ne "0" ]; then
   echo >&2 "Error checking nauty."
   exit 1
fi

comment:6 follow-up: Changed 5 years ago by jdemeyer

  • Status changed from needs_review to needs_info

Why fork nauty?

comment:7 follow-up: Changed 5 years ago by jdemeyer

And where does this version "26b17" come from?

comment:8 in reply to: ↑ 7 ; follow-up: Changed 5 years ago by dimpase

Replying to jdemeyer:

And where does this version "26b17" come from?

https://github.com/dimpase/nauty_sagemath/blob/master/This_is_nauty26b17.

so this is still a beta, but we'll update as soon as we get a release

comment:9 Changed 5 years ago by jdemeyer

Typo: distrubting

comment:10 in reply to: ↑ 8 ; follow-up: Changed 5 years ago by jdemeyer

comment:11 in reply to: ↑ 6 ; follow-ups: Changed 5 years ago by dimpase

Replying to jdemeyer:

Why fork nauty?

cause that's what Brendan would like - he want to keep releasing with his original license, I suppose.

comment:12 in reply to: ↑ 10 Changed 5 years ago by dimpase

Replying to jdemeyer:

Replying to dimpase:

https://github.com/dimpase/nauty_sagemath/blob/master/This_is_nauty26b17.

404 page not found...

the dot at the end is part of the URL

comment:13 in reply to: ↑ 11 ; follow-up: Changed 5 years ago by jdemeyer

Replying to dimpase:

Replying to jdemeyer:

Why fork nauty?

cause that's what Brendan would like - he want to keep releasing with his original license, I suppose.

I don't think that justifies forking nauty. Why not just ship the official tarball and add Brendan's permission as a separate file in the repo (say, build/pkgs/nauty/LICENSE).

comment:14 in reply to: ↑ 11 ; follow-up: Changed 5 years ago by jdemeyer

Replying to dimpase:

he want to keep releasing with his original license, I suppose.

For the record, does he know that is completely pointless since the nauty-in-Sage gives everybody permission to copy the code under the weaker license?

comment:15 in reply to: ↑ 13 ; follow-up: Changed 5 years ago by dimpase

Replying to jdemeyer:

Replying to dimpase:

Replying to jdemeyer:

Why fork nauty?

cause that's what Brendan would like - he want to keep releasing with his original license, I suppose.

I don't think that justifies forking nauty. Why not just ship the official tarball and add Brendan's permission as a separate file in the repo (say, build/pkgs/nauty/LICENSE).

there is no official tarball of this version available (we got the tarball by email), and the licence is in a .in file to be run with the tarball.

comment:16 in reply to: ↑ 14 Changed 5 years ago by dimpase

Replying to jdemeyer:

Replying to dimpase:

he want to keep releasing with his original license, I suppose.

For the record, does he know that is completely pointless since the nauty-in-Sage gives everybody permission to copy the code under the weaker license?

I don't read his mind, but I imagine that a professor of CS in one of the best universities in Australia knows what he is doing.

comment:17 in reply to: ↑ 15 ; follow-up: Changed 5 years ago by jdemeyer

Replying to dimpase:

there is no official tarball of this version available (we got the tarball by email)

That's a bit fishy. Can't you use the official tarball (+ patches if needed)?

comment:18 Changed 5 years ago by git

  • Commit changed from 076a8b5ceb7df52d62bccc09ed71a3bb099e248d to 400bc1114bf6a21f421534a7f40c52a395c9cbdf

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

400bc11package type and source repo, and a typo fix

comment:19 in reply to: ↑ 17 ; follow-up: Changed 5 years ago by dimpase

Replying to jdemeyer:

Replying to dimpase:

there is no official tarball of this version available (we got the tarball by email)

That's a bit fishy. Can't you use the official tarball (+ patches if needed)?

Now the gihub repo has the 26b17 updates over the version 25r9 we have (and which is the current publicly available version) You can take a look at a long diff...

comment:20 Changed 5 years ago by git

  • Commit changed from 400bc1114bf6a21f421534a7f40c52a395c9cbdf to 2a690f8d9ad22a4483d871ace7ec8b57ee2764f8

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

2a690f8package type and source repo, and a typo fix

comment:21 in reply to: ↑ 19 ; follow-up: Changed 5 years ago by nbruin

Replying to dimpase:

Now the gihub repo has the 26b17 updates over the version 25r9 we have (and which is the current publicly available version) You can take a look at a long diff...

I've tried. The relevant diffs would be between 26b17 and your repository, but I can't figure out how to do that in github (the main problem is that 26b17 doesn't seem to be available in your repository).

I think the reason why Jeroen is a little hesitant of using a fork of nauty (it's certainly why I am) is maintainability. What process for upgrading to the next version of nauty do you propose?

The usual way is to start with "pristine" upstream and patch that. It may be doable to do this with git instead, but in that case I think the procedure would be to rebase your changes to 26b17, not merge 26b17 into your branch (which seems to be what happened now). In that case you'd have to ship your (limited) git history in the tarball as a certificate of what changes have been made relative to upstream, and to give maintainers in the future the means to perform an upgrade in a relatively automated fashion.

comment:22 in reply to: ↑ 21 Changed 5 years ago by dimpase

Replying to nbruin:

Replying to dimpase:

Now the gihub repo has the 26b17 updates over the version 25r9 we have (and which is the current publicly available version) You can take a look at a long diff...

I've tried. The relevant diffs would be between 26b17 and your repository, but I can't figure out how to do that in github (the main problem is that 26b17 doesn't seem to be available in your repository).

26b17 is the 2nd commit, 25r9 is the 1st commit. Diff is too big to be viewed online, thus check the repo out and do git diff HEAD^ I'll mention this in SPKG.txt.

I think the reason why Jeroen is a little hesitant of using a fork of nauty (it's certainly why I am) is maintainability. What process for upgrading to the next version of nauty do you propose?

the relevant license file is in nauty-h.in. Just make sure that whenever you pour over the next nauty release, the relevant part of the file containing the license stays intact. Suggestions how to make this foolproof are welcome. But releases are not frequent at all, so I am not worried.

The usual way is to start with "pristine" upstream and patch that. It may be doable to do this with git instead, but in that case I think the procedure would be to rebase your changes to 26b17, not merge 26b17 into your branch (which seems to be what happened now). In that case you'd have to ship your (limited) git history in the tarball as a certificate of what changes have been made relative to upstream, and to give maintainers in the future the means to perform an upgrade in a relatively automated fashion.

there is no "my" branch. The repo is the released version 25r9 patched by 26b17.

comment:23 Changed 5 years ago by dimpase

The git repo in question should be in https://github.com/sagemath and not in https://github.com/dimpase

If we agree that this should go forward at all then I'll do this.

comment:24 Changed 5 years ago by git

  • Commit changed from 2a690f8d9ad22a4483d871ace7ec8b57ee2764f8 to 5b17a29fa79743c1eaa6548332a20a2f4a3efe77

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

ec8c74bMerge branch 'public/nauty26' of trac.sagemath.org:sage into nau
5b17a29upgrade to the GPL-compatible version 26r1

comment:25 Changed 5 years ago by dimpase

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

good to go, I think.

comment:26 Changed 5 years ago by dimpase

  • Description modified (diff)

comment:27 follow-up: Changed 5 years ago by fbissey

Should you install the new tools from this version? May not be useful in sage but as a general matter unless there is a reason I think we should install everything. This is my list

copyg listg labelg dretog amtog geng complg showg NRswitchg \
		biplabg addedgeg deledgeg countg pickg genrang newedgeg catg genbg \
		directg gentreeg genquarticg \
		ranlabg multig planarg gentourng linegraphg watercluster2 dretodot \
		subdivideg vcolg delptg cubhamg twohamg hamheuristic converseg \
		genspecialg genbgL \
		copyg listg labelg dretog amtog geng complg showg NRswitchg \
		dreadnaut

I'll sort it at some point.

comment:28 Changed 5 years ago by dimpase

  • Status changed from needs_review to needs_work
  • Work issues set to remove "#optional - nauty" tags

comment:29 in reply to: ↑ 27 Changed 5 years ago by dimpase

Replying to fbissey:

Should you install the new tools from this version? May not be useful in sage but as a general matter unless there is a reason I think we should install everything. This is my list

copyg listg labelg dretog amtog geng complg showg NRswitchg \
		biplabg addedgeg deledgeg countg pickg genrang newedgeg catg genbg \
		directg gentreeg genquarticg \
		ranlabg multig planarg gentourng linegraphg watercluster2 dretodot \
		subdivideg vcolg delptg cubhamg twohamg hamheuristic converseg \
		genspecialg genbgL \
		copyg listg labelg dretog amtog geng complg showg NRswitchg \
		dreadnaut

I'll sort it at some point.

it's quite a handful; I should have read the announcement more carefully.

comment:30 Changed 5 years ago by fbissey

I had to go to the makefile to make a proper list. And it looks like I got duplicates in the previous one so care is needed there.

all : nauty gtools ;

nauty : dreadnaut nauty.a nauty1.a nautyW.a nautyW1.a nautyL.a nautyL1.a;

gtools : copyg listg labelg dretog amtog geng complg showg NRswitchg \
  biplabg addedgeg deledgeg countg pickg genrang newedgeg catg genbg \
  directg gentreeg genquarticg \
  ranlabg multig planarg gentourng linegraphg watercluster2 dretodot \
  subdivideg vcolg delptg cubhamg twohamg hamheuristic converseg \
  genspecialg genbgL shortg ;

comment:31 Changed 5 years ago by git

  • Commit changed from 5b17a29fa79743c1eaa6548332a20a2f4a3efe77 to a64cc23fa0ead449d41b6c158bc98d922df75eb5

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

e200753removed #optional nauty-tags and checks whether nauty is installed
a64cc23install all progs

comment:32 Changed 5 years ago by dimpase

  • Status changed from needs_work to needs_review
  • Work issues remove "#optional - nauty" tags deleted

OK, all fixed, thanks for pointing these issues out.

comment:33 Changed 5 years ago by fbissey

  • Reviewers set to François Bissey

Apart from the issue to moving to standard it is all ok from me, I am ready to put in positive review if we have a clear signal on "standard".

comment:34 Changed 5 years ago by jdemeyer

  • Status changed from needs_review to needs_work
  • Work issues set to HTTP 404
Not Found

The requested URL /~coml0531/sage/nauty-26r1.tar.bz2 was not found on this server.

comment:35 Changed 5 years ago by jdemeyer

One detail: you don't need to copy the whole license to SPKG.txt, just a link should be sufficient, perhaps mention that it is GPL-compatible.

comment:36 Changed 5 years ago by dimpase

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

oops, wrong suffix in the URL. Fixed now.

comment:37 Changed 5 years ago by jdemeyer

  • Status changed from needs_review to needs_work
  • Work issues HTTP 404 deleted

In src/sage/graphs/graph_generators.py:

    Also: see the use of the optional nauty package for generating graphs
    at the :meth:`nauty_geng` method.

comment:38 Changed 5 years ago by jdemeyer

Author name...

comment:39 Changed 5 years ago by git

  • Commit changed from a64cc23fa0ead449d41b6c158bc98d922df75eb5 to 74d5ab954bcfe870db8b588a2309db1dd27e6f50

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

a5536bbMerge branch 'develop' of trac.sagemath.org:sage into develop
a1b0953Merge branch 'develop' of trac.sagemath.org:sage into develop
0c0e046Merge branch 'develop' of trac.sagemath.org:sage into develop
39e8d33Merge branch 'develop' of trac.sagemath.org:sage into develop
a3e0d4cMerge branch 'develop' of trac.sagemath.org:sage into develop
3c9e0aaMerge branch 'develop' of trac.sagemath.org:sage into develop
97723cfMerge branch 'develop' of trac.sagemath.org:sage into develop
c1bcd55Merge branch 'develop' of trac.sagemath.org:sage into develop
96156e8Merge remote-tracking branch 'trac/public/nauty26' into nau
74d5ab9addressed the reviewer's questions

comment:40 Changed 5 years ago by dimpase

  • Authors set to Dima Pasechnik

comment:41 Changed 5 years ago by git

  • Commit changed from 74d5ab954bcfe870db8b588a2309db1dd27e6f50 to 56815d4da72ff750813797d9158a82b1de9cd199

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

56815d4addressed the reviewer's questions

comment:42 Changed 5 years ago by dimpase

  • Status changed from needs_work to needs_review

OK, done (and then fixed spurrious merge commits that were not needed...)

comment:43 Changed 5 years ago by jdemeyer

  • Reviewers changed from François Bissey to François Bissey, Jeroen Demeyer
  • Status changed from needs_review to positive_review

comment:44 Changed 5 years ago by vbraun

  • Branch changed from public/nauty26 to 56815d4da72ff750813797d9158a82b1de9cd199
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.