#18564: Boost Edge Connectivity
-------------------------------------+-------------------------------------
Reporter: borassi | Owner: borassi
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-6.8
Component: graph theory | Resolution:
Keywords: Boost, | Merged in:
connectivity | Reviewers:
Authors: Michele Borassi | Work issues:
Report Upstream: N/A | Commit:
Branch: | 75cdbb6786b2cdce82715a56bcbb6924de317cf4
u/borassi/boost_edge_connectivity | Stopgaps:
Dependencies: |
-------------------------------------+-------------------------------------
Changes (by ncohen):
* status: needs_review => needs_work
Comment:
Hello Michele,
Your patch is rather big, and I think that it can be simplified:
- There is a weird 'pass' in boost_graph.pxd
- `#clang c++` is not useful in Sage's source
- You should add your new file to the doc index
(http://doc.sagemath.org/html/en/developer/sage_manuals.html#adding-a
-new-file)
Also, the header of your new module should describe what it implements
- This will make you notice (Sphinx will scream) that whatever follows a
'::'
should be indented.
- A module's documentation is no place for 'todo'. If you want to work on
something, create a trac ticket.
- Why a static 'from_sage_graph' function instead of a constructor
accepting a
graph as an input?
- `sig_on/sig_off` -- these markers are used so that the code can be
interrupted
while some C code is being run. Why would you put them around functions
which
end instantaneously, like `add_edge/add_vertex`?
- The variable 'i' is also an integer, and you should define it as 'cdef
int' so
that your Cython code will be better optimised.
- Why would you define `edge_connectivity_boost_*` functions in
`generic_graph_pyx` instead of doing it in the `graph_boost` file? You
can
import them if needed.
- Why did you move the LP out of the current `edge_connectivity` function?
Can't
you leave if where it is, and at the beginning of the file call the
boost
functions if this is what the user wants?
- When you submit a branch, always look at the final 'diff'. You can see
it by
clicking on the (green) branch's name on its ticket, and it will tell
you what
the reviewer sees and how he will have to figure out what your code
does. Everything that is not crystal clear for you is not crystal clear
for
him.
Good evening,
Nathann
--
Ticket URL: <http://trac.sagemath.org/ticket/18564#comment:25>
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.
