Opened 5 years ago
Last modified 16 months ago
#15381 new defect
Comparison of morphisms assumes that a Morphism is determined by its action on gens()
Reported by: | darij | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.4 |
Component: | categories | Keywords: | categories, gens, morphisms, modules |
Cc: | tscrim, caruso, SimonKing, nthiery, mmezzarobba | Merged in: | |
Authors: | Reviewers: | ||
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #10963 | Stopgaps: | wrongAnswerMarker |
Description (last modified by )
Counterexamples:
sage: from sage.categories.morphism import SetMorphism sage: f = SetMorphism(Hom(QQ, QQ, Sets()), numerator) sage: f.is_identity() True
and
sage: D3 = GroupAlgebra(DihedralGroup(3), QQ) sage: from sage.categories.modules_with_basis import * sage: g = ModuleMorphismByLinearity(domain=D3, codomain=D3, on_basis=lambda x: (D3.zero() if list(x) == [] else D3.basis()[x])) sage: g.is_identity() True
Of course, g
is not the identity. The culprit is here:
gens = domain.gens() for x in gens: if self(x) != x: return False return True
This is part of the is_identity
method in sage/categories/morphism.pyx
. The assumption is that the gens
method and the morphism refer to the same category, but here they don't: the morphism is a module morphism, while D3.gens()
refers to the generators as algebra.
Note that the equality check takes the other extreme and seems to only return True
if the on_basis
lambda functions of both morphisms are identical (i. e., I can add zero to each image and it doesn't return True
anymore, even if they are identical).
Change History (12)
comment:1 Changed 5 years ago by
comment:2 follow-up: ↓ 6 Changed 5 years ago by
- Cc SimonKing nthiery added
- Dependencies set to #10963
Battle plan:
- Wait for #10963 to be merged.
- Define
monoid_gens(self)
,group_gens(self)
,module_gens(self, base_ring)
,ring_gens(self)
andalgebra_gens(self, base_ring)
.
- For every (relevant) category
C
, define a category-level methodC.object_gens(object)
that callsobject.[whatever]_gens()
where[whatever]
is the name of the category.
- Redefine
gens(self)
to only work in the case whenself
is DEFINED by generators and relations: for example, ifself
is defined as a polynomial ring (or a quotient thereof), thengens(self)
should be the (projections of the) indeterminates; but whenself
is (say) a group algebra,gens(self)
shouldn't be defined at all. For the sake of deprecation, don't actually throw errors but rather return the old result with a deprecation warning.
What do you think?
comment:3 Changed 5 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:4 Changed 5 years ago by
- Cc mmezzarobba added
comment:5 Changed 5 years ago by
From emails between Nicolas and me:
[Nicolas] Do you see other occurrences of this situation? Does it need to be highlighted further right now? This topic is already touched a bit in the discussion about "the order of super categories".
[Darij] "hom" (as in "PolynomialRing?(QQ, 'x').hom" or "SymmetricGroup?(3).hom") is likely to lead to similar problems (not surprisingly as it relies on "gen" -- it should be supplanted at the same time as "gen"). In contrast, "Hom" (a method to generate the homspace rather than a single homomorphsim) is implemented semi-reasonably (it takes a "category" variable, but unfortunately it ducktypes it if it is not provided, which allows for writing fragile code). Also, this is precisely the type of issue I saw coming with Quotients -- although it was mainly a guess since I don't know what infrastructure will actually come for those. More likely it won't be "Quotients" but "quotient" (on objects of categories) causing troubles, since the "quotient of X by I" depends on what category we consider X to be in. I'm not sure if "extension", "cartesian_product" and the likes will be problematic (depends on how widely they are used). I would rather like to have these issues stressed in the documentation.
[Nicolas] Agreed!!! They should all accept a category argument if they don't yet, and/or be split in separate methods like for algebra_generators and friends. And developers should specify the category explicitly whenever there is an ambiguity (e.g. in generic code). I would not call "ducktyping" the default category for hom(A,B) though: the semantic is fully specified from A.category() and B.category(), and those are set explicitly by A and B. It's just that you'd better know A and B well to predict the result :-)
Actually, for hom, you need not only a category argument but also an argument to specify how the morphism should be computed, as the two things may differ. You typically may want to implement a Hopf algebra morphism by linearity. For now we have a couple specialized methods like "module_morphism", but the more systematic plan to tackle this is briefly stated on:
[Darij]
Agreed!!! They should all accept a category argument if they don't yet, and/or be split in separate methods like for algebra_generators and friends. And developers should specify the category explicitly whenever there is an ambiguity (e.g. in generic code). I would not call "ducktyping" the default category for hom(A,B) though: the semantic is fully specified from A.category() and B.category(), and those are set explicitly by A and B. It's just that you'd better know A and B well to predict the result :-)
Yeah, but some constructors decide on the category of the object they construct at runtime, based on conditions like whether some parameter given is invertible or not. The category hierarchy is probably not at fault here; just saying that things will go wrong every once in a while.
comment:6 in reply to: ↑ 2 Changed 5 years ago by
Replying to darij:
Battle plan:
- Wait for #10963 to be merged.
- Define
monoid_gens(self)
,group_gens(self)
,module_gens(self, base_ring)
,ring_gens(self)
andalgebra_gens(self, base_ring)
.
For the record: we already have semigroup_generators
,
monoid_generators
, group_generators
, algebra_generators
,
basis
, etc. What we need is a more systematical use and
advertisement of them. In particular it should be made clear that gens
is nothing but a short hand for casual interactive use, and should
*not* be used in code.
module_generators
, algebra_generators
and the like need not take a
base ring, since they are relative to the distinguished choice of base
ring in the parent.
- For every (relevant) category
C
, define a category-level methodC.object_gens(object)
that callsobject.[whatever]_gens()
where[whatever]
is the name of the category.
- Redefine
gens(self)
to only work in the case whenself
is DEFINED by generators and relations: for example, ifself
is defined as a polynomial ring (or a quotient thereof), thengens(self)
should be the (projections of the) indeterminates; but whenself
is (say) a group algebra,gens(self)
shouldn't be defined at all. For the sake of deprecation, don't actually throw errors but rather return the old result with a deprecation warning.
A weaker step would be to just define gens
in each category as an
alias to the generators for this category. With that, gens would refer
to the most specific one. Nothing super robust, but sufficient for
interactive use.
By the way: it's best if all the '*_generators' methods would return families.
Cheers,
Nicolas
comment:7 Changed 5 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:8 Changed 5 years ago by
- Milestone changed from sage-6.3 to sage-6.4
comment:9 follow-up: ↓ 10 Changed 4 years ago by
Related (duplicate?): #17768
comment:10 in reply to: ↑ 9 Changed 4 years ago by
Replying to mmezzarobba:
Related (duplicate?): #17768
Strongly related, but not a duplicate as this could potentially be an issue in other parts of Sage.
comment:11 Changed 2 years ago by
- Stopgaps set to wrongAnswerMarker
comment:12 Changed 16 months ago by
- Description modified (diff)
- Summary changed from gens() can mean both module and algebra generators, confusing morphism.pyx to Comparison of morphisms assumes that a Morphism is determined by its action on gens()
Thinking about it, I'm not sure if
gens
can be trusted at all, particularly in the case of algebras over different ground rings. Radical suggestion: deprecate it in favor ofgroup_gens(self)
,module_gens(self, base_ring)
,ring_gens(self)
andalgebra_gens(self, base_ring)
?