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

Graph.is_eulerian fails when the graph contains a connected component with non-comparable vertices #35168

Closed
2 tasks done
cyrilbouvier opened this issue Feb 21, 2023 · 1 comment · Fixed by #35170
Closed
2 tasks done

Comments

@cyrilbouvier
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues for a bug report that matches the one I want to file, without success.

Did you read the documentation and troubleshoot guide?

  • I have read the documentation and troubleshoot guide

Environment

- **OS**: Debian 11
- **Sage Version**: 9.8

Steps To Reproduce

G = Graph ([[0, 42, 'John'], [(42, 0)]])
print (G.is_eulerian()) 
H = Graph ([[0, 42, 'John'], [(42, 'John')]])
print (H.is_eulerian())

Expected Behavior

False
False

Actual Behavior

False
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In [4], line 1
----> 1 H.is_eulerian()

File ~/src/sage-9.8/src/sage/graphs/generic_graph.py:4123, in GenericGraph.is_eulerian(self, path)
   4120 # unconnected graph can still be Eulerian if all components
   4121 # up to one doesn't contain any edge
   4122 nontrivial_components = 0
-> 4123 for cc in self.connected_components():
   4124     if len(cc) > 1:
   4125         nontrivial_components += 1

File ~/src/sage-9.8/src/sage/graphs/connectivity.pyx:169, in sage.graphs.connectivity.connected_components()
    167 for v in G:
    168     if v not in seen:
--> 169         c = connected_component_containing_vertex(G, v, sort=sort)
    170         seen.update(c)
    171         components.append(c)

File ~/src/sage-9.8/src/sage/graphs/connectivity.pyx:287, in sage.graphs.connectivity.connected_component_containing_vertex()
    285 
    286     if sort:
--> 287         c.sort()
    288     return c
    289 

File ~/src/sage-9.8/src/sage/rings/integer.pyx:916, in sage.rings.integer.Integer.__richcmp__()
    914     c = mpz_cmp_d((<Integer>left).value, d)
    915 else:
--> 916     return coercion_model.richcmp(left, right, op)
    917 
    918 return rich_to_bool_sgn(op, c)

File ~/src/sage-9.8/src/sage/structure/coerce.pyx:2008, in sage.structure.coerce.CoercionModel.richcmp()
   2006     raise bin_op_exception('<=', x, y)
   2007 elif op == Py_GT:
-> 2008     raise bin_op_exception('>', x, y)
   2009 else:
   2010     raise bin_op_exception('>=', x, y)

TypeError: unsupported operand parent(s) for >: 'Integer Ring' and '<class 'str'>'

Additional Information

When a graph contains vertices that are non comparable (like integers and strings) in the same connected components, calling is_eulerian on this graph raises a TypeError.

dcoudert added a commit to dcoudert/sage that referenced this issue Feb 22, 2023
@dcoudert dcoudert mentioned this issue Feb 22, 2023
4 tasks
@dcoudert
Copy link
Contributor

Thanks for reporting this bug. The fix is rather simple.

vbraun pushed a commit that referenced this issue Mar 26, 2023
    
### 📚 Description

Fixes #35168.

We set parameter `sort` to `False` when calling `connected_components`.
This fixes the bug as we avoid sorting vertices with different types.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->

- [x] I have made sure that the title is self-explanatory and the
description concisely explains the PR.
- [x] I have linked an issue or discussion.
- [x] I have created tests covering the changes.
- [x] I have updated the documentation accordingly.

### ⌛ Dependencies
None
    
URL: #35170
Reported by: David Coudert
Reviewer(s): Marc Mezzarobba
vbraun pushed a commit that referenced this issue Jun 3, 2023
… vertices

    
### 📚 Description

As illustrated by the following code, the method `adjacency_matrix` of
the GenericGraph class failed when vertices where not sortable.

```python
G = Graph()
G.add_vertices ([14, 'test'])
G.adjacency_matrix()
```

I fixed the problem by using `sort=False` instead of `sort=True` in the
line that get the list of vertices.

I did not open an issue for this bug, but it is similar to #35168 (fixed
by PR #35170)


**NOTE**
I started to write a test to cover the changes but all others test are
broken by this small commit. Indeed the adjacency matrix obtained with
the new code can have a different row/column order. I am not sure what
is the correct way to handle this situation

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->

- [x] I have made sure that the title is self-explanatory and the
description concisely explains the PR.
- [x] I have linked an issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies
    
URL: #35245
Reported by: cyrilbouvier
Reviewer(s): cyrilbouvier, David Coudert
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants