Skip to content
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

Implement reverse() for LaurentSeries #25219

Closed
BrentBaccala opened this issue Apr 20, 2018 · 23 comments
Closed

Implement reverse() for LaurentSeries #25219

BrentBaccala opened this issue Apr 20, 2018 · 23 comments

Comments

@BrentBaccala
Copy link

Power series have a reverse() method for series of valuation 1.

This enhancement wraps the power series method and makes it available for LaurentSeries of valuation 1.

sage: R.<x> = Frac(QQ[['x']])
sage: f = 2*x + 3*x^2 - x^4 + O(x^5)
sage: g = f.reverse()
sage: g
1/2*x - 3/8*x^2 + 9/16*x^3 - 131/128*x^4 + O(x^5)
sage: f(g)
x + O(x^5)
sage: g(f)
x + O(x^5)

Component: algebra

Author: Brent Baccala

Branch/Commit: ebd8f45

Reviewer: Vincent Delecroix

Issue created by migration from https://trac.sagemath.org/ticket/25219

@BrentBaccala BrentBaccala added this to the sage-8.2 milestone Apr 20, 2018
@BrentBaccala
Copy link
Author

Commit: d7ed828

@BrentBaccala
Copy link
Author

Author: Brent Baccala

@BrentBaccala
Copy link
Author

Branch: u/gh-BrentBaccala/25219

@BrentBaccala
Copy link
Author

New commits:

d7ed828Trac #25219: add reverse() method to LaurentSeries

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 9, 2018

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

90ab7edMerge tag '8.4' into u/gh-BrentBaccala/25219

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 9, 2018

Changed commit from d7ed828 to 90ab7ed

@BrentBaccala BrentBaccala modified the milestones: sage-8.2, sage-8.5 Dec 18, 2018
@videlec
Copy link
Contributor

videlec commented Mar 4, 2019

comment:5

Rather than type(self)(self._parent, rev) couldn't you use the parent call method? (something like self._parent(rev)?

@videlec
Copy link
Contributor

videlec commented Mar 4, 2019

comment:6

Also, I am not able to reproduce the bug that you mention in the ticket description on 8.7.beta6...

@videlec
Copy link
Contributor

videlec commented Mar 4, 2019

Reviewer: Vincent Delecroix

@videlec videlec modified the milestones: sage-8.5, sage-8.7 Mar 4, 2019
@embray
Copy link
Contributor

embray commented Mar 25, 2019

comment:7

Ticket retargeted after milestone closed (if you don't believe this ticket is appropriate for the Sage 8.8 release please retarget manually)

@embray embray modified the milestones: sage-8.7, sage-8.8 Mar 25, 2019
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 12, 2019

Changed commit from 90ab7ed to c2e486c

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 12, 2019

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

c450c74Merge tag '8.7' into u/gh-BrentBaccala/25219
c2e486cTrac #25219: simplify method call syntax when creating reversed Laurent series

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 12, 2019

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

e86ed66Trac #25219: add test case

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 12, 2019

Changed commit from c2e486c to e86ed66

@BrentBaccala

This comment has been minimized.

@BrentBaccala
Copy link
Author

comment:10

Code updated as suggested.

I can't reproduce the bug anymore, either. I added it as a test case.

@videlec
Copy link
Contributor

videlec commented Apr 12, 2019

comment:11

Does power series really behave this way

+        if rev.parent() == u.parent():
+            return self._parent(rev)
+        else:
+            P = self._parent.change_ring(rev.parent().base_ring())
+            return P(rev)

It is very counterintuitive to have methods whose output type depends on the nature of the answer. If you do 4/2 in Sage you do not obtain an integer but a rational number. I don't understand why .reverse() should be any different.

I would suggest to have reverse and reverse_unit.

I am just asking for your opinion as this is beyond the scope of the ticket.

@BrentBaccala
Copy link
Author

comment:12

Replying to @videlec:

Does power series really behave this way

+        if rev.parent() == u.parent():
+            return self._parent(rev)
+        else:
+            P = self._parent.change_ring(rev.parent().base_ring())
+            return P(rev)

It is very counterintuitive to have methods whose output type depends on the nature of the answer. If you do 4/2 in Sage you do not obtain an integer but a rational number. I don't understand why .reverse() should be any different.

I would suggest to have reverse and reverse_unit.

I am just asking for your opinion as this is beyond the scope of the ticket.

I chose to mimic the behavior of the existing reverse method in PowerSeries_poly. Its docstring states:

If the leading coefficient is not a unit, we pass to its fraction field if possible

The Laurent series code calls the power series code, and follows its lead. So the answer to your question is yes, power series really do behave this way:

sage: R.<x> = ZZ[[]]
sage: (2*x+x^2).parent()
Power Series Ring in x over Integer Ring
sage: (2*x+x^2).reverse()
1/2*x - 1/8*x^2 + 1/16*x^3 - 5/128*x^4 + 7/256*x^5 - 21/1024*x^6 + 33/2048*x^7 - 429/32768*x^8 + 715/65536*x^9 - 2431/262144*x^10 + 4199/524288*x^11 - 29393/4194304*x^12 + 52003/8388608*x^13 - 185725/33554432*x^14 + 334305/67108864*x^15 - 9694845/2147483648*x^16 + 17678835/4294967296*x^17 - 64822395/17179869184*x^18 + 119409675/34359738368*x^19 + O(x^20)
sage: (2*x+x^2).reverse().parent()
Power Series Ring in x over Rational Field

I would tend to think that it should work this way, that 4/2 should return an integer and only 3/2 should return a rational, but I haven't thought about it enough to have a really informed opinion.

@videlec
Copy link
Contributor

videlec commented Apr 14, 2019

comment:13

The following test is working for me

sage: B.<b> = PolynomialRing(ZZ)
sage: A.<t> = LaurentSeriesRing(B)
sage: f = 2*b*t + b*t^2 + 3*b^2*t^3 + O(t^4)
sage: g = f.reverse()
sage: g(f)  # known bug - f fails to coerce properly, but next test works
t + O(t^4)

Why is it discarded?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 14, 2019

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

ebd8f45Trac #25219: remove TEST that was already an EXAMPLE

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 14, 2019

Changed commit from e86ed66 to ebd8f45

@BrentBaccala
Copy link
Author

comment:15

Replying to @videlec:

The following test is working for me

sage: B.<b> = PolynomialRing(ZZ)
sage: A.<t> = LaurentSeriesRing(B)
sage: f = 2*b*t + b*t^2 + 3*b^2*t^3 + O(t^4)
sage: g = f.reverse()
sage: g(f)  # known bug - f fails to coerce properly, but next test works
t + O(t^4)

Why is it discarded?

It used to fail. I don't know what changed to fix it, but it was somewhere else in Sage. I'll bisect the code if you really want to track it down.

Once it was working, I added it as a test case, but forgot that it was already there as a "known bug" example. Fixed.

@vbraun
Copy link
Member

vbraun commented Apr 15, 2019

Changed branch from u/gh-BrentBaccala/25219 to ebd8f45

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants