Opened 9 years ago

# Equality of Factorizations

Reported by: Owned by: caruso tbd minor sage-6.4 factorization equality factorization caruso N/A

### Description

Currently, two Factorization object are considered to be equal if they have the same value, but a Factorization object is not equal to its value:

```sage: P.<x> = QQ[]
sage: a = x+1; b = x+2; c = x+3
sage: F = Factorization([(a,1),(b,1),(c,1)]); F
(x + 1) * (x + 2) * (x + 3)
sage: F1 = Factorization([(a*b,1),(c,1)]); F1
(x + 3) * (x^2 + 3*x + 2)
sage: F2 = Factorization([(a,1),(b*c,1)]); F2
(x + 1) * (x^2 + 5*x + 6)
sage: F == F1
True
sage: F == F2
True
sage: F1 == F2
True
sage: F == a*b*c
False
```

Is this normal? (It seems to me really weird.)

PS: Am I supposed to ask this kind of questions here or is there a better place?

### comment:1 Changed 9 years ago by caruso

• Cc caruso added

### comment:2 Changed 8 years ago by jdemeyer

• Milestone changed from sage-5.11 to sage-5.12

### comment:3 Changed 8 years ago by chapoton

The methods `__eq__` and `__ne__` seems not to be implemented in the class.

### comment:4 Changed 8 years ago by caruso

I would happily implement them but I'm wondering what should be the correct behaviour of these methods. Do you have an opinion? (Don't forget that a ` Factorization` object can be "non commutative" as well.)

### comment:5 Changed 8 years ago by chapoton

well, one could try the following in the comparison of self and other

1) look if other is in self.parent (or maybe do that for the first factors)

2) if not compare (the value of self) and other

3) if yes, ask if self.parent is_commutative

4) if yes, sort and compare ; if not compare without sorting ?

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

here is a patch with a proposal for `__eq__`

### comment:7 Changed 8 years ago by caruso

The patch looks ok to me (but I also think that I'm not a good reviewer).

I'm just wondering whether we really want this:

```sage: factor(691*(x-4)*(x+6)) == factor(691*(y-4)*(y+6))
False
```

Indeed, even if 691 is a unit in one case and is not in the other case, the factorizations are the same, aren't they?

### comment:8 Changed 8 years ago by chapoton

Well, one must choose the meaning of equality.

One precise meaning would be

• a factorisation is made of a unit and a set/list of factors
• two factorisations are equal if they have the same unit and the same factors (up to order maybe)

Another possible solution would be to consider the unit as just any other factor. I do no like this solution, and I prefer the previous (current) behavior

If you want, you can maybe ask for opinions on sage-devel.

Something else: I have just thought that it would sometimes be good to separate factors in the center of the ring (commuting) and other factors, and to implement "partially commutative factorisations". But this is something for the wishlist.

### comment:9 Changed 7 years ago by vbraun_spam

• Milestone changed from sage-6.1 to sage-6.2

### comment:10 Changed 7 years ago by vbraun_spam

• Milestone changed from sage-6.2 to sage-6.3

### comment:11 Changed 7 years ago by vbraun_spam

• Milestone changed from sage-6.3 to sage-6.4

### comment:12 Changed 7 years ago by chapoton

• Branch set to u/chapoton/13186
• Commit set to 84b36a2056754713bc05056b28e7bb9ab86b0f24
• Status changed from new to needs_review

New commits:

 ​84b36a2 `trac 13186 comparison of factorisations`

### comment:13 Changed 7 years ago by vdelecroix

• Status changed from needs_review to needs_work

Hello,

Please add specifications to the doc! It is not clear to me (and to you as well looking at comment:8) what should be an equality between element.

I really found weird the begining

```if not isinstance(other, Factorization):
return self.value() == other
```

how can you compare a factorization with something else. Would you like that

```sage: [1,2,3] == (1,2,3)
True
```

Instead of using `Sequence`, what about

```sage: from sage.structure.element import get_coercion_model
sage: cm = get_coercion_model()
sage: cm.common_parent(1,1/2)
Rational field
```

(not so important, as at the end of the day the same code is called)

If the ring is commutative your test is wrong as the elements of the underlying universe are not necessarily sortable... you can not assume that the order is the same based on the fact that you call `sorted`. It might also depend on the specification you will add...

You need to define a `__ne__` if you want that `!=` works as well. Basically it would be

```def __ne__(self, other):
return not (self == other)
```

Vincent

### comment:14 Changed 7 years ago by nbruin

There are other problems too:

```sage: R.<x>=QQ[]
sage: hash(x^2)
15360174650385708
sage: hash(factor(x^2))
-5999452984666080493
sage: factor(x^2).value()==x^2
True
```

If you want to equate factorizations with their values (a reasonable thing to do) then their hashes should be equal too. I'd think a reasonable thing to do is to set

```factor(A) == B iff factor(A).value() == B
hash(factor(A)) == hash(factor(A).value())
```

and in fact to implement them by punting to the tests on values.

Note that if "factorizations" are ever returned in domains that do not have unique factorizations, this might be confusing: after all, there are supposed to be factorizations of the same value into irreducibles there that are not mutually equal.

### comment:15 Changed 4 years ago by chapoton

• Branch u/chapoton/13186 deleted
• Commit 84b36a2056754713bc05056b28e7bb9ab86b0f24 deleted
• Status changed from needs_work to needs_info

There is now a `__richcmp__` method in this class. One needs to check if the issue still stands.

Last edited 4 years ago by chapoton (previous) (diff)
Note: See TracTickets for help on using tickets.