Opened 7 years ago
Last modified 21 months ago
#13186 needs_info defect
Equality of Factorizations
Reported by:  caruso  Owned by:  tbd 

Priority:  minor  Milestone:  sage6.4 
Component:  factorization  Keywords:  equality factorization 
Cc:  caruso  Merged in:  
Authors:  Reviewers:  
Report Upstream:  N/A  Work issues:  
Branch:  Commit:  
Dependencies:  Stopgaps: 
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?
Attachments (1)
Change History (16)
comment:1 Changed 7 years ago by
 Cc caruso added
comment:2 Changed 6 years ago by
 Milestone changed from sage5.11 to sage5.12
comment:3 Changed 6 years ago by
comment:4 Changed 6 years ago by
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 6 years ago by
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 ?
Changed 6 years ago by
comment:6 Changed 6 years ago by
here is a patch with a proposal for __eq__
comment:7 Changed 6 years ago by
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*(x4)*(x+6)) == factor(691*(y4)*(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 6 years ago by
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 sagedevel.
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 5 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:10 Changed 5 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:11 Changed 5 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:12 Changed 5 years ago by
 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 5 years ago by
 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 5 years ago by
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 21 months ago by
 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.
The methods
__eq__
and__ne__
seems not to be implemented in the class.