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

Fixes a bug in is_chordal -- two algorithms #11961

Closed
nathanncohen mannequin opened this issue Oct 29, 2011 · 7 comments
Closed

Fixes a bug in is_chordal -- two algorithms #11961

nathanncohen mannequin opened this issue Oct 29, 2011 · 7 comments

Comments

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Oct 29, 2011

This ticket follows #11735, which fixes the bug reported by Jan on sage-devel [1]. It makes his alternative algorithm available in the function, so that we now have two different versions available, and still double-check the values it returns :-)

Requires : #11735

Apply : attachment: trac_11961.patch

[1] https://groups.google.com/d/topic/sage-support/rU1VTz1Ou_I/discussion

Component: graph theory

Author: Nathann Cohen, Jan Elffers

Reviewer: David Coudert

Merged: sage-4.8.alpha5

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

@nathanncohen nathanncohen mannequin added this to the sage-4.8 milestone Oct 29, 2011
@nathanncohen

This comment has been minimized.

@nathanncohen nathanncohen mannequin added the s: needs review label Oct 29, 2011
@jdemeyer
Copy link

jdemeyer commented Nov 3, 2011

Milestone sage-4.7.3 deleted

@jdemeyer jdemeyer removed this from the sage-4.8 milestone Nov 3, 2011
@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Nov 6, 2011

Attachment: trac_11961.patch.gz

@dcoudert
Copy link
Contributor

comment:3

I had no problem when installing the patch. The doc is also correctly generated.

All tests I have performed are consistent: both algorithms return the same boolean value. The certificate are not the same when several certificates are possible.

Algorithm B is a bit slower than algorithm A.

As I already said for patch #11735, significant running time improvements are possible for these linear time algorithms, but at least we have some algorithms ;)

Thank you Nathann.

@dcoudert
Copy link
Contributor

Reviewer: David Coudert

@dcoudert dcoudert added this to the sage-4.8 milestone Dec 10, 2011
@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Dec 10, 2011

comment:4

Thank you for the review ! :-)

Algorithm B is a bit slower than algorithm A.

Yep. That's why it is good to have both :-)

As I already said for patch #11735, significant running time improvements are possible for these linear time algorithms, but at least we have some algorithms ;)

These codes are really standard. It would really be a shame not to implement them properly in C/C++ at some point.

Nathann

@jdemeyer
Copy link

Merged: sage-4.8.alpha5

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

3 participants