Opened 11 years ago

Closed 11 years ago

#5242 closed enhancement (fixed)

[with patch, positive review] generic_power can now handle semi-groups

Reported by: hivert Owned by: hivert
Priority: major Milestone: sage-3.3
Component: algebra Keywords: generic power
Cc: sage-combinat Merged in:
Authors: Reviewers:
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by mabshoff)

The former implementation of generic_power_c made two assumptions

  • not a is well defined for any element a
  • There is a one

These two assumptions do not always always hold. So I reorganized the initial code which handle particular cases so that:

  • it does not call "not a" when the exponent n is different from zero
  • it delegates to ~a the responsibility of raising an error when the exponent n is negative and a is not invertible. This allows for monoids with partially defined inversion (e.g. the multiplicative monoid of Z/12Z).

This modification revealed a problem in matrices: the empty matrix 0x0 modulo 2 was tested to be non invertible! I changed the test to the correct value.

Investigating this revealed many other inconsistencies with matrices; those will be taken care of in a subsequent patch.

Attachments (1)

generic_power_fix_semigroup-5242-submitted.patch (2.5 KB) - added by hivert 11 years ago.
Proposal of patch for generic power

Download all attachments as: .zip

Change History (11)

Changed 11 years ago by hivert

Proposal of patch for generic power

comment:1 Changed 11 years ago by mabshoff

  • Milestone set to sage-combinat

comment:2 Changed 11 years ago by mabshoff

  • Description modified (diff)

comment:3 Changed 11 years ago by hivert

  • Owner changed from tbd to hivert
  • Status changed from new to assigned

comment:4 Changed 11 years ago by hivert

  • Milestone changed from sage-combinat to sage-3.3

comment:5 follow-up: Changed 11 years ago by cremona

I would prefer not to have implicit casting of general types to booleans. Can we not have "if n==0" or "if n.is_zero()" instead of "if not n", and similar? Remember that Sage code is all visible to users, so we should avoid obscure programming idioms.

comment:6 in reply to: ↑ 5 ; follow-up: Changed 11 years ago by hivert

Replying to cremona: Yes Sure. I more than agree with you. Remember that I didn't wrote this code, I just reorganize the order in which the tests are done. I'll soon propose another patch with a better idiom.

comment:7 in reply to: ↑ 6 Changed 11 years ago by hivert

Replying to cremona: Actually it seems that for n it is possible because we know its type. On the contrary it is not possible for a. Indeed a can be a plain Python int (wich forbid the use of a.is_zero()) of any type eg polynomial with forbid the use of == 0 so I don't know what else to do (I'm far from being a cython expert) :

 sage -t  "devel/sage-combinat/sage/rings/polynomial/polydict.pyx"
**********************************************************************
File "/usr/local/sage/devel/sage-combinat/sage/rings/polynomial/polydict.pyx", line 695:
    sage: (f-f)**0
Expected:
    Traceback (most recent call last):
    ...
    ArithmeticError: 0^0 is undefined.
Got:
    PolyDict with representation {0: 1}

So if there is no better suggestion I'll stick with the current patch.

comment:8 Changed 11 years ago by mabshoff

I just tested my current 3.3.rc1 merge tree with this patch and

All tests passed!

Cheers,

Michael

comment:9 Changed 11 years ago by malb

  • Summary changed from [with patch, needs review] generic_power can now handle semi-groups to [with patch, positive review] generic_power can now handle semi-groups

Patch reads good, I suggest to open another ticket for the style issue.

comment:10 Changed 11 years ago by mabshoff

  • Cc sage-combinat added
  • Resolution set to fixed
  • Status changed from assigned to closed

Merged in Sage 3.3.rc1.

Cheers,

Michael

Note: See TracTickets for help on using tickets.