Equality checking for ActionDigraph

[MTWhyte]

On my current fpsemi-examples branch, where I import the re

from libsemigroups_pybind11 import Sims1, congruence_kind, presentation
from libsemigroups_pybind11.fpsemigroup import make, full_transformation_monoid

p = make(full_transformation_monoid(4))
p.alphabet(5)
presentation.replace_word(p, [], [4])
presentation.add_identity_rules(p, 4)

C = Sims1(congruence_kind.right)
C.short_rules(p)

k = C.number_of_congruences(2)
it = C.iterator(2)
graphs = []
for i in range(k):
    graphs.append(next(it))

I find that graphs[0] == graphs[1] is True. This suggests that equality checking for action digraphs possibly hadn't been implemented. I wondered if ActionDigraphs which give equal strings as output from their repr are 'equal' under ==, but the following illustrates this isn't true:

In [48]: ad1 = ActionDigraph(2)
In [49]: ad2 = ActionDigraph(2)
In [50]: ad1.add_to_out_degree(2)
In [51]: ad2.add_to_out_degree(2)
In [52]: ad1.add_edge(0, 0, 0)
In [53]: ad1.add_edge(0, 1, 1)
In [54]: ad1.add_edge(1, 0, 0)
In [55]: ad1.add_edge(1, 1, 1)
In [56]: ad2.add_edge(0, 0, 1)
In [57]: ad2.add_edge(0, 1, 0)
In [58]: ad2.add_edge(1, 0, 1)
In [59]: ad2.add_edge(1, 1, 0)
In [60]: ad1
Out[60]: <action digraph with 2 nodes, 4 edges, 2 out-degree>
In [61]: ad2
Out[61]: <action digraph with 2 nodes, 4 edges, 2 out-degree>
In [62]: ad1 == ad2
Out[62]: False

There is possibly something unexpected going on here, or something just as of yet unimplemented, but I'm not sure exactly what.

[james-d-mitchell]

Thanks @MTWhyte. The digraphs ad1 and ad2 in your second code block aren't equal, so why do you think the answer is wrong? Their __repr__ being equal doesn't mean anything, there are plenty of non-equal (and non-isomorphic) graphs (even action/word graphs) with the same numbers of nodes and edges.

The first issue your having is the same as #70, the iterator points at a reference to a single graph, so when the iterator is incremented, the value of graphs[0] is the same as that for graphs[1]. I'm not sure that this is exactly a bug, maybe more of an issue with the documentation.

If you do:

[ActionDigraph(x) for x in C.iterator(2)]

In your example, you copy x (which is changed in-place when calling next) using the copy constructor of ActionDigraph. There are good reasons to not do this copying by default (performance!). So I think this becomes an issue in the documentation rather than in the code itself.