sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.44k stars 480 forks source link

Graph library passes almost surelly erroneous calls without an exception #12540

Open 15606e31-f165-4fa5-94d5-bd9bfb0e1636 opened 12 years ago

15606e31-f165-4fa5-94d5-bd9bfb0e1636 commented 12 years ago
  1. Dense backend doesn't support edge labels, but add_edge(1, 2, 3) passes and behaves like add_edge(1, 2, None). (#12387)
  2. Adding a loop to a graph with allows_loops=False do nothing.
  3. Adding an already present edge to a graph with allows_multiedges=False do nothing.
  4. (Other?)

Is this the proper behavior?

CC: @vbraun @nathanncohen

Component: graph theory

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

pelegm commented 7 years ago
comment:5

No answer here, so I ask explicitly again about (2).

In SparseGraphBackend, for example, the following lines appear in the add_edge method:

        if (not self.loops(None)) and u_int == v_int:
            return

This means that if the backend does not allow loops, the add_edge method silently ignore the added edge. By default, SparseGraphBackend does not allow loops.

Is this a wanted behaviour?

If not, there are couple of ways to handle this:

  1. raise from inside the backend method
  2. warn (somehow) from inside the backend method
  3. let the backend return False (or whatever) in case the edge was not added, and let the graph itself notice it and raise/warn
dcoudert commented 2 years ago

Description changed:

--- 
+++ 
@@ -1,5 +1,5 @@
 1. Dense backend doesn't support edge labels, but add_edge(1, 2, 3) passes and behaves like add_edge(1, 2, None). (#12387)
-2. Adding a loop to a graph with allows_loops=False do nothing.
+2. ~~Adding a loop to a graph with allows_loops=False do nothing.~~
 3. Adding an already present edge to a graph with allows_multiedges=False do nothing.
 4. (Other?)
dcoudert commented 2 years ago
comment:7

The behavior for loops has change and the default behavior is now to raise an error by default.

sage: G = Graph([(0, 0)], loops=None, data_structure="sparse")
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
...
ValueError: cannot add edge from 0 to 0 in graph without loops
sage:
sage: G = Graph([(0, 0)], loops=None, data_structure="dense")
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
...
ValueError: cannot add edge from 0 to 0 in graph without loops
sage:
sage: G = Graph([(0, 0)], loops=False, data_structure="sparse")
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
...
ValueError: cannot add edge from 0 to 0 in graph without loops
sage:
sage: G = Graph([(0, 0)], loops=False, data_structure="dense")
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
...
ValueError: cannot add edge from 0 to 0 in graph without loops