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 )
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)
Change History (11)
Changed 11 years ago by
comment:1 Changed 11 years ago by
- Milestone set to sage-combinat
comment:2 Changed 11 years ago by
- Description modified (diff)
comment:3 Changed 11 years ago by
- Owner changed from tbd to hivert
- Status changed from new to assigned
comment:4 Changed 11 years ago by
- Milestone changed from sage-combinat to sage-3.3
comment:5 follow-up: ↓ 6 Changed 11 years ago by
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: ↓ 7 Changed 11 years ago by
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
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
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
- 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
- Cc sage-combinat added
- Resolution set to fixed
- Status changed from assigned to closed
Merged in Sage 3.3.rc1.
Cheers,
Michael
Proposal of patch for generic power