Opened 4 years ago

Last modified 4 years ago

#24247 closed enhancement

Implement __pow__ in the coercion model — at Version 45

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-8.2
Component: coercion Keywords:
Cc: Merged in:
Authors: Jeroen Demeyer Reviewers:
Report Upstream: N/A Work issues:
Branch: u/jdemeyer/implement___pow___in_the_coercion_model (Commits, GitHub, GitLab) Commit: c5b1be1372141cf3447ea4c28208b60aeaa3ae03
Dependencies: #24467 Stopgaps:

Status badges

Description (last modified by jdemeyer)

We implement powering in the coercion model. One important difference between powering and other operators is that the most common use case for powering is powering something to an integer exponent.

To deal with this integer powering, we implement an action IntegerPowAction. This action calls a special method _pow_int() on the element. In other words, x ^ n for an integer n becomes x._pow_int(n). We also provide a default implementation of _pow_int for MonoidElement and RingElement which uses the square-and-multiply algorithm implemented in generic_power().

For backward compatibility reasons, we also call this action for elements of IntegerModRing(m). In the future, we may rethink what to do here.

Apart from this, powering behaves like other binary operators: coercion to a common parent is done if no action is defined.

Note that the 3-argument version of pow() is not supported in the coercion model. Only specific types like Integer implement it.

We also fix various serious bugs in powering for RDF such as:

sage: RDF(0) ^ RDF(-1)
0.0
sage: RDF(-1) ^ RDF(2)
NaN

Change History (45)

comment:1 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:2 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:3 Changed 4 years ago by jdemeyer

  • Dependencies changed from #24244 to #24248

comment:4 Changed 4 years ago by jdemeyer

  • Branch set to u/jdemeyer/implement___pow___in_the_coercion_model

comment:5 Changed 4 years ago by git

  • Commit set to a18280f20d1a3a9d87300be699b5efa77ac10d8b

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

9ad8983Fake Integer interface
04235beNew functions integer_check_long() and integer_check_long_py()
ad34554Fix isinstance(x, int) calls in element.pyx
a18280fImplement __pow__ in the coercion model

comment:6 Changed 4 years ago by git

  • Commit changed from a18280f20d1a3a9d87300be699b5efa77ac10d8b to 8b194fb82d37eb5036f36ee37cf6b38bc27e0d30

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

bb114c9Fake Integer interface
4d76bc1New functions integer_check_long() and integer_check_long_py()
9c579c7Fix isinstance(x, int) calls in element.pyx
8b194fbImplement __pow__ in the coercion model

comment:7 Changed 4 years ago by git

  • Commit changed from 8b194fb82d37eb5036f36ee37cf6b38bc27e0d30 to 54eb896167c809daad138588f9bc86b5c5659591

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

54eb896Implement __pow__ in the coercion model

comment:8 Changed 4 years ago by jdemeyer

  • Dependencies changed from #24248 to #24248, #24259

comment:9 Changed 4 years ago by git

  • Commit changed from 54eb896167c809daad138588f9bc86b5c5659591 to 5e031351b06fe41b299b1f2fb74cb294f3489fb1

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

5e03135Implement __pow__ in the coercion model

comment:10 Changed 4 years ago by jdemeyer

  • Dependencies changed from #24248, #24259 to #24248, #24259, #24260

comment:11 Changed 4 years ago by git

  • Commit changed from 5e031351b06fe41b299b1f2fb74cb294f3489fb1 to 0d3aa638f654dbc104ec85887eedfdb13e271589

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

88eaed5Declare Integer.value as array
5c1f43fDeprecate str ^ Integer
70592a8Fix compiler warning
d3db645Merge commit '88eaed585d8159a4a0763e56caaf3a767d4cb2f0'; commit '5c1f43f861165ff975616f60cab37c92d36d10f6'; commit '70592a850d7677928592c022760d6183bd81364f' into t/24247/implement___pow___in_the_coercion_model
0d3aa63Implement __pow__ in the coercion model

comment:12 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:13 Changed 4 years ago by git

  • Commit changed from 0d3aa638f654dbc104ec85887eedfdb13e271589 to 09afe6dd0312ea51fdb636440cab21f3cc6eccd6

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

09afe6dImplement __pow__ in the coercion model

comment:14 Changed 4 years ago by jdemeyer

  • Dependencies changed from #24248, #24259, #24260 to #24248, #24259, #24260, #24275, #24276

comment:15 Changed 4 years ago by git

  • Commit changed from 09afe6dd0312ea51fdb636440cab21f3cc6eccd6 to b0b4070a17e0ac455b569e3f37ace754abda7c89

Branch pushed to git repo; I updated commit sha1. New commits:

cabd346Merge tag '8.1.rc4' into t/24247/implement___pow___in_the_coercion_model
b0b4070Various fixes

comment:16 Changed 4 years ago by jdemeyer

  • Dependencies changed from #24248, #24259, #24260, #24275, #24276 to #24248, #24259, #24260, #24275, #24276, #24328

comment:17 Changed 4 years ago by git

  • Commit changed from b0b4070a17e0ac455b569e3f37ace754abda7c89 to f6d7ba296b19249e4b25a014ce5d3b6ea4986a9e

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

c0aca1cNew module to implement generic_power
bcb70b3Merge commit '70592a850d7677928592c022760d6183bd81364f'; commit '88eaed585d8159a4a0763e56caaf3a767d4cb2f0'; commit '5c1f43f861165ff975616f60cab37c92d36d10f6'; commit 'c0aca1c0ac92b95d790f39369a683269efbde530' into t/24247/implement___pow___in_the_coercion_model
f6d7ba2Implement __pow__ in the coercion model

comment:18 Changed 4 years ago by jdemeyer

  • Dependencies changed from #24248, #24259, #24260, #24275, #24276, #24328 to #24328, #24259, #24260, #24275, #24276

comment:19 Changed 4 years ago by git

  • Commit changed from f6d7ba296b19249e4b25a014ce5d3b6ea4986a9e to 7658aa46f07c76703ff6dc2e5598abd440306f7a

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

fb9a0c2Various fixes related to generic_power()
de1c6f6Merge commit '70592a850d7677928592c022760d6183bd81364f'; commit '88eaed585d8159a4a0763e56caaf3a767d4cb2f0'; commit '5c1f43f861165ff975616f60cab37c92d36d10f6'; commit 'fb9a0c2e5a0dd88e82b7b6f1f56846deb696a56b' into t/24247/implement___pow___in_the_coercion_model
7658aa4Implement __pow__ in the coercion model

comment:20 Changed 4 years ago by git

  • Commit changed from 7658aa46f07c76703ff6dc2e5598abd440306f7a to edca7ab9da69ea2659f48cadbeb4915888f586f5

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

edca7abImplement __pow__ in the coercion model

comment:21 Changed 4 years ago by jdemeyer

  • Dependencies changed from #24328, #24259, #24260, #24275, #24276 to #24260, #24275, #24350

comment:22 Changed 4 years ago by jdemeyer

  • Dependencies #24260, #24275, #24350 deleted

comment:23 Changed 4 years ago by git

  • Commit changed from edca7ab9da69ea2659f48cadbeb4915888f586f5 to 4c442dc8a9f88811d0208c9d71f65a07162dd961

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

4c442dcImplement __pow__ in the coercion model

comment:24 Changed 4 years ago by git

  • Commit changed from 4c442dc8a9f88811d0208c9d71f65a07162dd961 to cd95bbcace8f654401862748df5158f0f77be0d6

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

cd95bbcImplement __pow__ in the coercion model

comment:25 Changed 4 years ago by git

  • Commit changed from cd95bbcace8f654401862748df5158f0f77be0d6 to 59a04410c0b0891d8836fe2f4a712cd21d38fa4d

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

59a0441Implement __pow__ in the coercion model

comment:26 Changed 4 years ago by jdemeyer

  • Status changed from new to needs_review

comment:27 Changed 4 years ago by git

  • Commit changed from 59a04410c0b0891d8836fe2f4a712cd21d38fa4d to 19e671bcea9f0e6dc3b3f3a629a7f64c2bd3795f

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

19e671bImplement __pow__ in the coercion model

comment:28 follow-up: Changed 4 years ago by nbruin

Isn't __pow__ just a right action, by either ZZ or RR?

comment:29 in reply to: ↑ 28 Changed 4 years ago by jdemeyer

Replying to nbruin:

Isn't __pow__ just a right action, by either ZZ or RR?

It could be an action. But I don't really understand what you are trying to say.

Are you saying that it should be an action?

Are you saying that there should not be a special method like _pow_int?

(Note that these are two independent questions)

Last edited 4 years ago by jdemeyer (previous) (diff)

comment:30 follow-up: Changed 4 years ago by nbruin

I was really just wondering if there would be a benefit to treating it as an action. There may be other hooks that are already available in the coercion framework, but the standard binary operation code tries (if I recall correctly) to find a *common* parent, and that's not the right thing to do for powering in general. Actions, on the other hand, should come pretty close. I have no feeling for the performance implications, though. I just raised the point because a brief scanning through the contributions here didn't seem to have considered this possibility.

For _pow_int: given the various powering strategies that exist for different types of rings (square-multiply, frobenius, power series expansion), my guess is that it's worth having a hook for it.

comment:31 in reply to: ↑ 30 Changed 4 years ago by jdemeyer

Replying to nbruin:

For _pow_int: given the various powering strategies that exist for different types of rings (square-multiply, frobenius, power series expansion), my guess is that it's worth having a hook for it.

OK good. So we are just talking about the code which calls this method.

comment:32 follow-ups: Changed 4 years ago by vdelecroix

Close to the arguments in comment:30, I am not sure that finding a common parent is a good solution to handle powerings. The only reasonable assumption that could be done with powering is (x^a)^b = x^(a*b) (that is: it is a multiplicative action).

Also, the command QQbar.gen()^(1/3) should not try to coerce 1/3 to QQbar. Rational, as integers, should definitely be treated apart. Though, do we have any concrete case where exponents can be something else than integer/rational? With respect to rational powering, this ticket raises an important question: what should be done with this coercion incoherence

sage: 4**(1/2)
2
sage: type(_)
<type 'sage.rings.rational.Rational'>
sage: 3**(1/2)
sqrt(3)
sage: type(_)
<type 'sage.symbolic.expression.Expression'>

Note also that x^a * y^a = (x*y)^a is wrong with rational exponents has often there is a choice of a root

sage: ((-1)^(1/3))^3
-1
sage: ((-1)^3)^(1/3)
(-1)^(1/3)

comment:33 in reply to: ↑ 32 Changed 4 years ago by nbruin

Replying to vdelecroix:

Also, the command QQbar.gen()^(1/3) should not try to coerce 1/3 to QQbar. Rational, as integers, should definitely be treated apart. Though, do we have any concrete case where exponents can be something else than integer/rational?

Yes, on anything with a real or complex analytic structure one probably wants that x^y is a shorthand for exp(log(x)*y) (although for serious use the latter is much better for understanding subtleties arising from multi-valuedness)

It does look like we might want to handle powering action by integers (no multi-valuedness problem, but may need existence or adjunction of inverses), by rationals, by appropriate completions of rationals as different actions, because of their different problems.

In principle, on a commutative multiplicative group of exponent N we'd even have a powering action by ZZ/N ZZ. I'm not so sure it's worth supporting that explicitly, though.

comment:34 Changed 4 years ago by jdemeyer

A lot of points are raised here.

First of all, the point of this ticket is making possible various ways of implementing powering in various parents, not to actually implement it that way. For example, I don't plan to re-implement powering for QQbar in this ticket. What is within the scope of this ticket is making it possible to implement a parent which treats powering by rationals in a specific way (by going through the coercion model which checks for actions and this QQbar^QQ powering should be an action).

Second, I do agree that powering by integers is special enough that there should be a default action (as suggested by Nils) which calls the special method _pow_int.

Third, I think that coercing to a common parent does make sense because:

  1. It is consistent with all other operators. Often multiplication is treated as an action too. Still, when no action is found, coercion to a common parent is done.
  1. There are several parents where a common parent for powering makes sense, in particular the various incarnations of real/complex numbers and the symbolic ring.
  1. If coercion to a common parent shouldn't be done, then what should be done by default if no action is found?

comment:35 in reply to: ↑ 32 Changed 4 years ago by jdemeyer

Replying to vdelecroix:

With respect to rational powering, this ticket raises an important question: what should be done with this coercion incoherence

sage: 4**(1/2)
2
sage: type(_)
<type 'sage.rings.rational.Rational'>
sage: 3**(1/2)
sqrt(3)
sage: type(_)
<type 'sage.symbolic.expression.Expression'>

Sorry, but that is a question about the implementation of powering for a specific parent, so not within the scope of this ticket.

comment:36 Changed 4 years ago by git

  • Commit changed from 19e671bcea9f0e6dc3b3f3a629a7f64c2bd3795f to 5b1435647c0d84a2a086afaa82e6359e77838b89

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

d61f9eaBruhatTitsTree.lift: bail out if matrix is not invertible
5d6c831New abstract base class IntegerAction
5b14356Implement __pow__ in the coercion model

comment:37 Changed 4 years ago by jdemeyer

  • Dependencies set to #24450

comment:38 Changed 4 years ago by jdemeyer

  • Milestone changed from sage-8.1 to sage-8.2
  • Status changed from needs_review to needs_work

comment:39 Changed 4 years ago by git

  • Commit changed from 5b1435647c0d84a2a086afaa82e6359e77838b89 to 186f2b88e64401027b9f347061afc0d8632ab345

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

56f2a64New abstract base class IntegerAction
186f2b8Implement __pow__ in the coercion model

comment:40 Changed 4 years ago by git

  • Commit changed from 186f2b88e64401027b9f347061afc0d8632ab345 to ec64a0e098309f31d6dc3e1181ccbc989ea6e1f4

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

4443adfNew abstract base class IntegerAction
ec64a0eImplement __pow__ in the coercion model

comment:41 Changed 4 years ago by git

  • Commit changed from ec64a0e098309f31d6dc3e1181ccbc989ea6e1f4 to f2a59b900c81749b29c3eca97c88fead8aac98dc

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

f2a59b9Implement __pow__ in the coercion model

comment:42 Changed 4 years ago by git

  • Commit changed from f2a59b900c81749b29c3eca97c88fead8aac98dc to 457bd56ec78916fb73c17baad7af93b59ab25b56

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

85cabe7Refactor IntegerMulAction
457bd56Implement __pow__ in the coercion model

comment:43 Changed 4 years ago by jdemeyer

  • Dependencies changed from #24450 to #24467

comment:44 Changed 4 years ago by git

  • Commit changed from 457bd56ec78916fb73c17baad7af93b59ab25b56 to c5b1be1372141cf3447ea4c28208b60aeaa3ae03

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

c5b1be1Implement __pow__ in the coercion model

comment:45 Changed 4 years ago by jdemeyer

  • Description modified (diff)
  • Status changed from needs_work to needs_review
Note: See TracTickets for help on using tickets.