Opened 9 years ago

Closed 2 years ago

#14068 closed defect (fixed)

nauty_geng is naughty

Reported by: azi Owned by: jason, ncohen, rlm
Priority: major Milestone: sage-9.0
Component: graph theory Keywords:
Cc: dimpase Merged in:
Authors: David Coudert, Dima Pasechnik Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: 93edfb0 (Commits, GitHub, GitLab) Commit: 93edfb05921b4fba999b9eda0c7f128d99be1f36
Dependencies: Stopgaps:

Status badges

Description (last modified by dimpase)

I think nauty_geng should not ignore geng's return value and return an exception if the returned value is not 0. Consider the following:

def find_counterexample(n):
   for G in graphs.nauty_geng("-c" + str(n)):
     if is_counterexample(G):
        print 'Yeees, there is a counterexample'
        return 
   print 'No counterexamples! Conjecture holds'

the code appears to be just fine but it has a bug! Namely it tries to run the program geng with the parameter "-cn" instead of "-c n". Hence geng returns an error and instead of getting an error in Sage, it simply does not generate any graphs.

The branch implements the catching of geng's error. We also clean up the code of graph_generators a bit.



Change History (14)

comment:1 Changed 8 years ago by jdemeyer

  • Cc changed from rbeezer,ncohen, dcoudert to rbeezer, ncohen, dcoudert
  • Milestone changed from sage-5.11 to sage-5.12

comment:2 Changed 8 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:3 Changed 8 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:4 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:5 Changed 2 years ago by dcoudert

  • Authors set to David Coudert
  • Branch set to public/graphs/14068_nauty
  • Commit set to 759fab2f8961aa5ab7e6d5e57ac96f9625f70080
  • Milestone changed from sage-6.4 to sage-8.9
  • Status changed from new to needs_review

A possible fix is to raise an error only when parameter debug is False and that something goes wrong.

When debug is True, the behavior remains the same as before and so we continue to display the message output by geng in stderr.


New commits:

759fab2trac #14068: check input when debug is False

comment:6 follow-up: Changed 2 years ago by dcoudert

I don't understand the reported pyflakes error with sage.graphs.strongly_regular_db. Other modules are imported the same way without causing pyflakes errors.

comment:7 Changed 2 years ago by dcoudert

  • Cc dimpase added; rbeezer ncohen dcoudert removed

comment:8 in reply to: ↑ 6 Changed 2 years ago by dimpase

Replying to dcoudert:

I don't understand the reported pyflakes error with sage.graphs.strongly_regular_db. Other modules are imported the same way without causing pyflakes errors.

perhaps pyflakes is confused by staticmethod() thing. The only place where a method from strongly_regular_db is used is

strongly_regular_graph = staticmethod(sage.graphs.strongly_regular_db.strongly_regular_graph)

by the way, I don't understand why it's not written as

from . import strongly_regular_db
...
strongly_regular_graph = staticmethod(strongly_regular_db.strongly_regular_graph)

Is it cause this code pre-dates from . syntax?

comment:9 Changed 2 years ago by git

  • Commit changed from 759fab2f8961aa5ab7e6d5e57ac96f9625f70080 to 93edfb05921b4fba999b9eda0c7f128d99be1f36

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

93edfb0refactoring using 'from .' syntax

comment:10 Changed 2 years ago by dimpase

OK, otherwise looks good. let's see if this commit makes pyflakes happy

comment:11 Changed 2 years ago by dcoudert

pyflakes is now happy !

comment:12 Changed 2 years ago by dimpase

  • Authors changed from David Coudert to David Coudert, Dima Pasechnik
  • Description modified (diff)
  • Reviewers set to Dima Pasechnik
  • Status changed from needs_review to positive_review

comment:13 Changed 2 years ago by chapoton

  • Milestone changed from sage-8.9 to sage-9.0

moving milestone to 9.0 (after release of 8.9)

comment:14 Changed 2 years ago by vbraun

  • Branch changed from public/graphs/14068_nauty to 93edfb05921b4fba999b9eda0c7f128d99be1f36
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.