-
-
Notifications
You must be signed in to change notification settings - Fork 528
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
a lazy solver for functional equations #37033
a lazy solver for functional equations #37033
Conversation
…; moving towards full coverage.
… of cache may contain variable, add failing test
….com/mantepse/sage into lazy_series/integration_experimental
…reams which are different
…s, use 'is' for equality when finding input streams in Stream_uninitialized
….com/mantepse/sage into lazy_series/integration_experimental
gR = None | ||
|
||
def coefficient(n): | ||
nonlocal gR | ||
r = R.zero() | ||
for i in range(n // gv + 1): | ||
# Make sure the element returned from the composition is in P | ||
r += P(self[i](g))[n] | ||
c = coeff_stream[i] | ||
if c in self.base_ring(): | ||
c = P(c) | ||
r += c[n] | ||
elif c.parent().base_ring() is self.base_ring(): | ||
r += c(g)[n] | ||
else: | ||
if gR is None: | ||
gR = [h.change_ring(c.parent().base_ring()) for h in g] | ||
r += c(gR)[n] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this change. Sorry if we've already discussed it and I don't remember.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️ I am grateful that you keep working on this ❤️!
No, I don't think we discussed this specifically. The reason for the hack (which I hate) is as follows:
- recall that
g
is a tuple of (at least 2) elements of lazy series. - if
c
is an element of the base ringB
,c(g)
cannot work correctly (and thus just returnc
) - e.g., integers are not callable, polynomials would give nonsense. - if
c
is a proper element of the_laurent_poly_ring
$R$ , we are in the easy case - otherwise,
c
should be an element of the modified_laurent_poly_ring
. E.g., If$R=\mathbb Q[x,y]$ , we have$c\in \mathbb Q[\vec a][x,y]$ (where$\vec a$ isFESDUMMY
). In this case, to make the substitution work, I make sure that the elements ofg
are lazy series over the modified base ring, i.e.$\mathbb Q[\vec a]$ .
I just saw that the error changed after #37916. We now have the following:
sage: U.<a> = InfinitePolynomialRing(QQ)
sage: R.<x,y> = U[]
sage: L.<z> = LazyPowerSeriesRing(QQ)
sage: c = x
sage: c(z, 0)
---------------------------------------------------------------------------
RuntimeError Traceback (most recent call last)
Cell In[7], line 1
----> 1 c(z, Integer(0))
File ~/sage/src/sage/rings/polynomial/multi_polynomial_element.py:182, in MPolynomial_element.__call__(self, *x, **kwds)
180 y = K(0)
181 for m, c in self.element().dict().items():
--> 182 y += c * prod(v ** e for v, e in zip(x, m) if e)
183 return y
...
RuntimeError: There is a bug in the coercion code in Sage.
Both x (=1) and y (=z) are supposed to have identical parents but they don't.
In fact, x has parent 'Integer Ring'
whereas y has parent 'Infinite polynomial ring in a over Lazy Taylor Series Ring in z over Rational Field'
Original elements 1 (parent Infinite polynomial ring in a over Rational Field) and z (parent Lazy Taylor Series Ring in z over Rational Field) and maps
<class 'sage.structure.coerce_maps.DefaultConvertMap_unique'> (map internal to coercion system -- copy before use)
Coercion map:
From: Infinite polynomial ring in a over Rational Field
To: Infinite polynomial ring in a over Lazy Taylor Series Ring in z over Rational Field
<class 'sage.categories.morphism.SetMorphism'> (map internal to coercion system -- copy before use)
Generic morphism:
From: Lazy Taylor Series Ring in z over Rational Field
To: Infinite polynomial ring in a over Lazy Taylor Series Ring in z over Rational Field
sage: fun() | ||
[<sage.data_structures.stream.Stream_exact object at 0x...>] | ||
""" | ||
closure = self.get_coefficient.__closure__ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code is so hacky and liable to pick up far more than it actually needs and likely could be quite slow because it needs to check potentially a lot of objects. I think we should consider caching the result. I would still return a list
for consistency, but with a warning to the user not to mutate it (since it isn't part of the public facing stuff, only a more serious developer will get into this). Although perhaps that is slightly dangerous even within our code...? Perhaps these that need to do some computations be cached and every such method returns a tuple
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code is only executed once for each operation when creating the power series. For example,
sage: L.<z> = LazyPowerSeriesRing(SR)
sage: G = L.undefined(0)
sage: L.define_implicitly([(G, [ln(2)])], [diff(G) - exp(-G(-z))])
sage: %prun G[:20]
532675 function calls (516627 primitive calls) in 0.815 seconds
Ordered by: internal time
ncalls tottime percall cumtime percall filename:lineno(function)
...
2 0.000 0.000 0.000 0.000 stream.py:1027(input_streams)
Co-authored-by: Travis Scrimshaw <clfrngrown@aol.com>
Co-authored-by: Travis Scrimshaw <clfrngrown@aol.com>
I have one other question, concerning equality of parents: should I be using "==" or "is"? For example, in for i0, i in enumerate(indices):
c = s._cache[i]
if self._coefficient_ring == self._base_ring:
if c.parent() == self._PF:
c = retract(subs(c, var, val))
if c.parent() is not self._base_ring:
good = m - i0 - 1
elif c.parent() == self._U:
c = c.map_coefficients(lambda e: subs(e, var, val))
try:
c = c.map_coefficients(lambda e: retract(e), self._base_ring)
except TypeError:
good = m - i0 - 1
s._cache[i] = c I actually think that I did it backwards here. I understand that computations could in principle return an element with parent that is not identical to the one I'm expecting. Do we have a |
…tion of polynomial ring (which shouldn't be necessary in a better designed InfinitePolynomialRing)
We modify `ModuleAction` to prefer coercion of the base ring over creating an action. More precisely, before this branch, we have ``` sage: G = PolynomialRing(QQ, "x") sage: S = PolynomialRing(MatrixSpace(QQ, 2), "x") sage: G.gen() * S.gen() [x 0] [0 x]*x ``` instead of ``` [1 0] [0 1]*x^2 ``` and ``` sage: G = PolynomialRing(QQ, "x") sage: S = PolynomialRing(InfinitePolynomialRing(QQ, "a"), "x") sage: G.gen() * S.an_element() x*x ``` instead of ``` x^2 ``` which is at least surprising. In the end, this is needed to make sagemath#37033 work. Authors: @tscrim and @mantepse. URL: sagemath#37916 Reported by: Martin Rubey Reviewer(s): Travis Scrimshaw
We modify `ModuleAction` to prefer coercion of the base ring over creating an action. More precisely, before this branch, we have ``` sage: G = PolynomialRing(QQ, "x") sage: S = PolynomialRing(MatrixSpace(QQ, 2), "x") sage: G.gen() * S.gen() [x 0] [0 x]*x ``` instead of ``` [1 0] [0 1]*x^2 ``` and ``` sage: G = PolynomialRing(QQ, "x") sage: S = PolynomialRing(InfinitePolynomialRing(QQ, "a"), "x") sage: G.gen() * S.an_element() x*x ``` instead of ``` x^2 ``` which is at least surprising. In the end, this is needed to make sagemath#37033 work. Authors: @tscrim and @mantepse. URL: sagemath#37916 Reported by: Martin Rubey Reviewer(s): Travis Scrimshaw
We modify `ModuleAction` to prefer coercion of the base ring over creating an action. More precisely, before this branch, we have ``` sage: G = PolynomialRing(QQ, "x") sage: S = PolynomialRing(MatrixSpace(QQ, 2), "x") sage: G.gen() * S.gen() [x 0] [0 x]*x ``` instead of ``` [1 0] [0 1]*x^2 ``` and ``` sage: G = PolynomialRing(QQ, "x") sage: S = PolynomialRing(InfinitePolynomialRing(QQ, "a"), "x") sage: G.gen() * S.an_element() x*x ``` instead of ``` x^2 ``` which is at least surprising. In the end, this is needed to make sagemath#37033 work. Authors: @tscrim and @mantepse. URL: sagemath#37916 Reported by: Martin Rubey Reviewer(s): Travis Scrimshaw
Permutation (sub)groups are good examples, but I think their group algebras might not fall under this. Hom spaces between equal-but-not-identical parents also have the same behavior IIRC. I'm not sure how many rings fall under this though. I would probably just be consistent with using |
OK, I can do that. Meanwhile, I discovered another oddity. There is a bug in
This doesn't follow the specification. However, if I change this, I seem to get coercion errors. I have to double check, though. Update: sorry, I think it actually does, because |
I'm afraid I found the problem. Consider, as an example, the lazy power series ring I admit I do not fully understand why this happens only rarely, and if so, it is unconsequential most of the time. Mostly, it seems to happen with exact series, where we do not make sure that their data ( However, at least sometimes it is sheer luck that everything works out. For example, in I am not sure how to proceed. Do you think it makes sense to have another meeting? |
Superseeded by #38108 |
We enable sage to solve systems of functional equations of lazy power series, generalizing the existing possibility of defining series recursively. For example:
Dependencies: #37451, #37687, #37736, #37766, #37761, #37916