#17647: Branch and Bound for vertex separation
-------------------------+-------------------------------------------------
Reporter: | Owner:
dcoudert | Status: needs_work
Type: | Milestone: sage-6.5
enhancement | Resolution:
Priority: major | Merged in:
Component: graph | Reviewers:
theory | Work issues:
Keywords: | Commit:
Authors: David | fdfe120bc438b041a7ff480a288f29d6c73b17e5
Coudert | Stopgaps:
Report Upstream: N/A |
Branch: |
public/17647b |
Dependencies: |
#17665 |
-------------------------+-------------------------------------------------
Changes (by ncohen):
* status: needs_review => needs_work
Comment:
Helloooooo again,
- What is the purpose of `lower_bound`? The documentation says that the
code is
trying to find an ordering whose cost is *lower* than `lower_bound`? On
what
exactly is it a lower bound?
Do you mean that the code will "return any ordering whose cost is lower
than
`lower_bound`, even if it has no proof that it is optimal?
- Do you think that you actually need `loc_b_neighborhood`? It feels like
all
you need is what you store in `bits_tmp`, i.e. the union of the prefix
and the
neighborhood. And you could drop one of your two tmp bitets.
- Are those two lines needed?
{{{
delta_i = max(current_cost, delta_i)
if delta_i < upper_bound:
}}}
It seems that you can already suppose that `current_cost<upper_bound`,
and you
checked several lines above that `delta_i < upper_bound`.
- I do not agree with this comment
{{{
if upper_bound <= lower_bound:
# Either we have optimal solution, or we are satisfied with
# current solution.
}}}
it seems that this code only deals with the case when 'we are satisfied
with
the current solution'.
- Is there any reason why you did not expose your function in the main
`vertex_separation` function? If you do, is there any reason why it
should not
be the default one? (if it is faster?..). Finally, note that if you do
make it
the default some doctests of your functions will have to be updated, as
you
would be comparing the BAB function with `vertex_separation` (which
would also
call the BAB).
With those comments and those from my previous message, I would say that
my
review is finished. We only have to settle those last remarks.
Thanks for your work!
Nathann
--
Ticket URL: <http://trac.sagemath.org/ticket/17647#comment:36>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.