Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#11928 closed defect (fixed)

Update Graph.clique_number to use MILP

Reported by: ncohen Owned by: jason, ncohen, rlm
Priority: major Milestone: sage-4.8
Component: graph theory Keywords:
Cc: dcoudert Merged in: sage-4.8.alpha0
Authors: Nathann Cohen Reviewers: David Coudert
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #11846 Stopgaps:

Description (last modified by jdemeyer)

As reported by Peter Mueller, the clique_number method should have been updated in #11846.

As this ticket has been reviewed, let us do it here !

Apply: trac_11928.patch

Nathann

Attachments (1)

trac_11928.patch (2.8 KB) - added by ncohen 7 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 7 years ago by ncohen

  • Status changed from new to needs_review

comment:2 Changed 7 years ago by dcoudert

  • Reviewers set to David Coudert
  • Status changed from needs_review to needs_work

This patch is working correctly with the default algorithm "Cliquer". For the "MILP" algorithm, the result is also correct.

However:

  • it should be written that path 11846 (maximum independent set using MILP) should also be installed
  • the description of the method needs work:

line 3610: cliquer -> Cliquer` line 3610: add MILP line 3617: it is written algorithm == "MILP" but previous lines are only Cliquer networkx. Please unify. line 3652: "algorithm = algorithm" is confusing. Is this the standard notation ? Please unify spaces before/after "=" and "=="

That's all for the moment ;-)

comment:3 Changed 7 years ago by ncohen

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

Hello !!!

The patch is updated !

  • About the "algorithm = algorithm" : writing algorithm = "MILP" is equivalent, the goal is just to forward the parameter to the next function. It happens a lot in the code, and it's less trouble for later if the parameters need to be changed.
  • I also fixed the alignment of the cliques entry in the documentation. The HTML was badly formatted because of that.

Thanks for the comments ! :-)

Nathann

Changed 7 years ago by ncohen

comment:4 Changed 7 years ago by dcoudert

  • Status changed from needs_review to positive_review

The documentation is now correctly formatted, and the patch works fine.

comment:5 Changed 7 years ago by jdemeyer

  • Dependencies set to #11846
  • Description modified (diff)

comment:6 Changed 7 years ago by jdemeyer

  • Merged in set to sage-4.7.3.alpha0
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:7 Changed 7 years ago by jdemeyer

  • Milestone sage-4.7.3 deleted

Milestone sage-4.7.3 deleted

comment:8 Changed 7 years ago by jdemeyer

  • Merged in changed from sage-4.7.3.alpha0 to sage-4.8.alpha0
  • Milestone set to sage-4.8
Note: See TracTickets for help on using tickets.