Opened 6 years ago

Last modified 23 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 jdemeyer)

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 6 years ago by darij

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 of group_gens(self), module_gens(self, base_ring), ring_gens(self) and algebra_gens(self, base_ring)?

comment:2 follow-up: Changed 6 years ago by darij

  • 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) and algebra_gens(self, base_ring).
  • For every (relevant) category C, define a category-level method C.object_gens(object) that calls object.[whatever]_gens() where [whatever] is the name of the category.
  • Redefine gens(self) to only work in the case when self is DEFINED by generators and relations: for example, if self is defined as a polynomial ring (or a quotient thereof), then gens(self) should be the (projections of the) indeterminates; but when self 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 6 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:4 Changed 6 years ago by mmezzarobba

  • Cc mmezzarobba added

comment:5 Changed 5 years ago by darij

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:

http://trac.sagemath.org/ticket/10668#comment:17

[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 nthiery

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) and algebra_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 method C.object_gens(object) that calls object.[whatever]_gens() where [whatever] is the name of the category.
  • Redefine gens(self) to only work in the case when self is DEFINED by generators and relations: for example, if self is defined as a polynomial ring (or a quotient thereof), then gens(self) should be the (projections of the) indeterminates; but when self 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 vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:8 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:9 follow-up: Changed 5 years ago by mmezzarobba

Related (duplicate?): #17768

comment:10 in reply to: ↑ 9 Changed 5 years ago by tscrim

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 3 years ago by jakobkroeker

  • Stopgaps set to wrongAnswerMarker

comment:12 Changed 23 months ago by jdemeyer

  • 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()
Note: See TracTickets for help on using tickets.