Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug in is_hamiltonian: wrong exceptions are caught #16210

Closed
nathanncohen mannequin opened this issue Apr 22, 2014 · 16 comments
Closed

Bug in is_hamiltonian: wrong exceptions are caught #16210

nathanncohen mannequin opened this issue Apr 22, 2014 · 16 comments

Comments

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Apr 22, 2014

sage: g = graphs.CycleGraph(10)
sage: g.allow_loops(True)
sage: g.add_edge(0,0)
sage: g.is_hamiltonian()
False

This happens because in traveling_salesman_problem the call to _scream_is_not_simple raises a ValueError when the graph is not simple, and the same exception is raised when the graph is not hamiltonian.

We can use the EmptySetError to differentiate them, as we cannot optimize anything on an empty set.

Nathann

Component: graph theory

Author: Nathann Cohen

Branch/Commit: 0174081

Reviewer: Vincent Delecroix

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

@nathanncohen nathanncohen mannequin added this to the sage-6.2 milestone Apr 22, 2014
@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Apr 22, 2014

Branch: u/ncohen/16210

@nathanncohen

This comment has been minimized.

@nathanncohen nathanncohen mannequin added the s: needs review label Apr 22, 2014
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 22, 2014

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

18122c5trac #16210: Bug in is_hamiltonian: wrong exceptions are caught

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 22, 2014

Commit: 18122c5

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 23, 2014

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

bf44a1ctrac #16210:: Merge into 6.2.rc0
d69303ctrac #16210: Broken doctest

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 23, 2014

Changed commit from 18122c5 to d69303c

@videlec

This comment has been minimized.

@videlec
Copy link
Contributor

videlec commented Apr 23, 2014

Reviewer: Vincent Delecroix

@videlec
Copy link
Contributor

videlec commented Apr 23, 2014

comment:4

Hi Nathann,

Why not a NonImplementedError (with a shorter message)?

Vincent

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Apr 23, 2014

comment:5

Because I am not sure that _scream_if_not_simple should always raise a NotImplementedError in all functions that call it.

What is the problem with long messages ? They give more meaningful information.

Nathann

@videlec
Copy link
Contributor

videlec commented Apr 23, 2014

Changed branch from u/ncohen/16210 to public/16210

@videlec
Copy link
Contributor

videlec commented Apr 23, 2014

comment:6

Let us go for long messages...

I simplified (I hope) the implementation of the function _scream... and correct the doctest in is_hamiltonian. Set it to positive review if you like it...

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 23, 2014

Changed commit from d69303c to 0174081

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 23, 2014

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

0174081simpler implementation + corrected doctest

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Apr 23, 2014

comment:8

AHahah. Well, it does more tests than in the first version, but I don't think this can ever become a very important problem :-P

Nathann

@vbraun
Copy link
Member

vbraun commented Apr 30, 2014

Changed branch from public/16210 to 0174081

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants