Changes between Version 18 and Version 35 of Ticket #25076


Ignore:
Timestamp:
04/02/18 14:47:08 (3 years ago)
Author:
SimonKing
Comment:

Replying to jdemeyer:

Replying to SimonKing:

It isn't fixed.

I just pushed a branch fixing exactly the problem that you described and you're not happy because it doesn't also fix another issue? Good luck fixing it your way then.

Your behaviour makes me speechless. Almost.

The ticket description is about using the short-cut _mul_long as a solution to the problem that Matrix*int didn't work. As it turns out #24742 allows Matrix*int, but it doesn't do it by enabling _mul_long (although _mul_long is supported in sage.structure.element).

So, given the poor performance I demonstrated, there is an issue here that goes beyond what was fixed in #24742, and it is related with what the ticket description of this ticket says.

In addition, I notice that the current implementation of _mul_long for Matrix_gfpn_dense doesn't convert an integer into GF(p^n,'x') by reduction modulo p, but by some conversion used by MeatAxe. For instance, 4 would be converted to the number x+1 in GF(9,'x'). I didn't notice it before since in my cohomology applications I work with matrices over prime fields, where the different conversions happen to coincide.

Hence, if this ticket is about to "Fix Matrix_gfpn_dense*int", then here one should

  1. re-enable the usage of _mul_long for matrix times int, because of the performance issue
  2. fix _mul_long of Matrix_gfpn_dense for non-prime fields.

I changed the ticket description accordingly.

Consequently, this ticket is by no means just a duplicate of #24742.

And even when you insist that it is a duplicate: What would be the point of closing this ticket as a duplicate (or maybe just adding a test) and then opening yet another ticket that deals with the two remaining issues?

Alternatively, I could just leave the issues alone and use ._mul_long directly in my cohomology code, to be independent of whatever implementation of coercion is currently used.

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #25076

    • Property Status changed from new to needs_work
    • Property Dependencies changed from to #24742
    • Property Keywords _mul_long added; meataxe coercion removed
  • Ticket #25076 – Description

    v18 v35  
    1 The following happens on some systems (why only some?) when !MeatAxe is used as a matrix backend:
     1The following previous bug was fixed by #24742:
    22{{{
    33sage: M = matrix(GF(9,'x'), 2,3)
     
    163163}}}
    164164
    165 The Python int 1 should or course not be converted into the matrix space (simply because that matrix space is not an algebra), but instead should be converted into the matrix' basering, so that _lmul_ can be called.
    166 
    167 Or even better: The shortcut _mul_long() should be used, which is supported by sage.structure.element but isn't used anywhere in sage.matrix except in sage.matrix.matrix_gfpn_dense.
    168 
    169 It could be that the actual error is in the coercion system, although it seems to occur only with the !MeatAxe backend:
    170 {{{
    171 sage: M = MatrixSpace(GF(9,'x'), 2,3, implementation="generic")()
    172 sage: M*int(1)
    173 [0 0 0]
    174 [0 0 0]
    175 }}}
     165What #24742 doesn't fix:
     1661. Multiplication of a matrix with a scalar that is given as an integer doesn't use _mul_long() should be used, which is supported by sage.structure.element but isn't used anywhere in sage.matrix except in sage.matrix.matrix_gfpn_dense.
     1672. The _mul_long implementation of Matrix_gfpn_dense is flawed, as it *should* do a coercion of the given integer into the underlying base ring (which is by computing the remainder modulo the modulus of the base ring), but currently uses a different way of conversion.