Opened 6 years ago

Last modified 6 years ago

#17544 new task

Use single-underscore methods in MatrixMorphism_abstract

Reported by: pbruin Owned by:
Priority: minor Milestone: sage-6.5
Component: coercion Keywords: matrix
Cc: jpflori, SimonKing Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #10513 Stopgaps:

Description

After moving modules to the new coercion model (#10513), we should make sage.modules.matrix_morphism.MatrixMorphism_abstract use single-underscore methods for addition, subtraction, scalar multiplication and composition.

Change History (4)

comment:1 follow-up: Changed 6 years ago by SimonKing

Note that at some point I tried to make morphisms use the coercion model with single-underscore methods. It miserably failed, because of a subtle P/Cython bug: If one creates a Python class that inherits from both ModuleElement and Map, then some cdef attributes of Map get confused with some cdef attributes of ModuleElement, even though P/Cython accepts the definition of the sub-class.

comment:2 in reply to: ↑ 1 Changed 6 years ago by pbruin

Replying to SimonKing:

Note that at some point I tried to make morphisms use the coercion model with single-underscore methods. It miserably failed, because of a subtle P/Cython bug: If one creates a Python class that inherits from both ModuleElement and Map, then some cdef attributes of Map get confused with some cdef attributes of ModuleElement, even though P/Cython accepts the definition of the sub-class.

I see, it only makes sense to implement single-underscore methods once we are able to inherit from ModuleElement as well as from Map. So I guess that means that this ticket has to wait until Cython supports multiple inheritance?

comment:3 follow-up: Changed 6 years ago by tscrim

The other (temporary) options are to construct a class ModuleElementMorphism which copies the necessary code or to write a wrapper Morphism class that just redirects to some underlying matrix. I think the former is a better approach until cython does multiple inheritance (which I don't think will happen anytime soon, perhaps even at all?) as it can be applied and subclassed to things like AlgebraElementMorphism (for things like the endomorphism ring).

comment:4 in reply to: ↑ 3 Changed 6 years ago by SimonKing

Replying to tscrim:

I think the former is a better approach until cython does multiple inheritance (which I don't think will happen anytime soon, perhaps even at all?)...

Note that I did not talk about a cdef class with two base classes. I am talking about one Python class inheriting from two cdef base classes, which are considered to have compatible layout, but still things get wrong.

I just tried to reconstruct the example, but it seemingly just worked. So, perhaps the problem is actually solved one way or the other. If I recall correctly, the problematic cdef attributes included domain or codomain---which has now changed, because of weak referencing. Perhaps this change has not only fixed a memory leak, but has also worked around the c layout problem.

Note: See TracTickets for help on using tickets.