Opened 6 years ago

Closed 6 years ago

# bug in inequality of words

Reported by: Owned by: mantepse major sage-7.4 combinatorics abergeron, saliola, slabbe, amyglen Sébastien Labbé Travis Scrimshaw N/A 6b8ac18 6b8ac18b165e0eb74e0554eb191af103d6eb3cd3

### Description

Using 7.4.beta 6:

```sage: shape1 = Word([1,2], alphabet=[1,2])
sage: shape2 = Word([1,1], alphabet=[1,2])
sage: shape1 == shape2
False
sage: shape1 != shape2
False
```

### comment:1 Changed 6 years ago by mantepse

However:

```sage: Word([1,1], alphabet=[1,2]) != Word([1,2], alphabet=[1,2])
True
```

???

### comment:2 Changed 6 years ago by tscrim

It seems to be an issue with `WordDatatype_char.__richcmp__` in that if the `cmp` is non-zero, it needs to include 3 (which is `!=`):

```         cdef int test = (<WordDatatype_char> self)._lexico_cmp(other)
if test < 0:
return op < 2 or op == 3
elif test > 0:
-           return op > 3
+           return op >= 3
else:
return op == 1 or op == 2 or op == 5
```

### comment:3 Changed 6 years ago by mantepse

Thank you Travis! This worked, but I must say that I hate python.

### comment:4 follow-up:  5 Changed 6 years ago by mantepse

Can you explain why it fails only when assigning to a variable?

### comment:5 in reply to:  4 Changed 6 years ago by slabbe

Looking at the fix suggested by Travis, I believe the problem is not the variable but what is on the left side vs right side: `A != B` versus `B != A`.

I guess the problem comes from the recent upgrade of cmp to richcmp for Python 3 support done in #21435.

Last edited 6 years ago by slabbe (previous) (diff)

### comment:6 Changed 6 years ago by slabbe

Hmmm, looking at this further. I do not think this bug was included by #21435. It seems to come from the original implementation of `WordDatatype_char`.

I can confirm this using sagecell which runs 7.3:

```shape1 = Word([1,2], alphabet=[1,2])
shape2 = Word([1,1], alphabet=[1,2])
print shape1 == shape2, "should be False"
print shape2 == shape1, "should be False"
print shape1 != shape2, "should be True"
print shape2 != shape1, "should be True"
print shape2 == shape2, "should be True"
print shape2 != shape2, "should be False"
print version()
```

returns

```False should be False
False should be False
False should be True
True should be True
True should be True
False should be False
SageMath version 7.3, Release Date: 2016-08-04
```

### comment:7 Changed 6 years ago by slabbe

Authors: → Sébastien Labbé → u/slabbe/21609 → 6b8ac18b165e0eb74e0554eb191af103d6eb3cd3 new → needs_review

New commits:

 ​6b8ac18 `21609: Fixing A!=B for words char when A > B`

### comment:8 Changed 6 years ago by mantepse

Thanks for fixing. I have a very minor suggestion. Wouldn't it be more readable to code

```        if test < 0:
return op == 0 or op == 1 or op == 3
elif test > 0:
return op == 3 or op == 4 or op == 5
else:
return op == 1 or op == 2 or op == 5
```

### comment:9 Changed 6 years ago by slabbe

Yes, I agree. But, a little bit less efficient if op == 5 when test >0 for instance...

I must leave now to take a train, I won't have acces to internet until monday.

### comment:10 Changed 6 years ago by tscrim

Reviewers: → Travis Scrimshaw needs_review → positive_review

I agree that it is a reasonable shortcut to sacrifice the slight bit of readability, but considering the comparison types are (arbitrarily) assigned integers, readability is already somewhat out the window...

Last edited 6 years ago by tscrim (previous) (diff)

### comment:11 Changed 6 years ago by vbraun

Branch: u/slabbe/21609 → 6b8ac18b165e0eb74e0554eb191af103d6eb3cd3 → fixed positive_review → closed
Note: See TracTickets for help on using tickets.