Opened 22 months ago
Closed 10 months ago
#25219 closed enhancement (fixed)
Implement reverse() for LaurentSeries
Reported by:  ghBrentBaccala  Owned by:  

Priority:  minor  Milestone:  sage8.8 
Component:  algebra  Keywords:  
Cc:  Merged in:  
Authors:  Brent Baccala  Reviewers:  Vincent Delecroix 
Report Upstream:  N/A  Work issues:  
Branch:  ebd8f45 (Commits)  Commit:  ebd8f4500c2cdc09d701ffab9105fe72e9f4506d 
Dependencies:  Stopgaps: 
Description (last modified by )
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)
Change History (17)
comment:1 Changed 22 months ago by
 Branch set to u/ghBrentBaccala/25219
 Commit set to d7ed82816e9274eebb570cc1f1697b35a39d81f3
comment:2 Changed 22 months ago by
 Status changed from new to needs_review
comment:3 Changed 14 months ago by
 Commit changed from d7ed82816e9274eebb570cc1f1697b35a39d81f3 to 90ab7ed44057b4a02b70f9b169632155fca22bd9
Branch pushed to git repo; I updated commit sha1. New commits:
90ab7ed  Merge tag '8.4' into u/ghBrentBaccala/25219

comment:4 Changed 14 months ago by
 Milestone changed from sage8.2 to sage8.5
comment:5 Changed 11 months ago by
Rather than type(self)(self._parent, rev)
couldn't you use the parent call method? (something like self._parent(rev)
?
comment:6 Changed 11 months ago by
 Milestone changed from sage8.5 to sage8.7
 Reviewers set to Vincent Delecroix
 Status changed from needs_review to needs_info
Also, I am not able to reproduce the bug that you mention in the ticket description on 8.7.beta6...
comment:7 Changed 10 months ago by
 Milestone changed from sage8.7 to sage8.8
Ticket retargeted after milestone closed (if you don't believe this ticket is appropriate for the Sage 8.8 release please retarget manually)
comment:8 Changed 10 months ago by
 Commit changed from 90ab7ed44057b4a02b70f9b169632155fca22bd9 to c2e486c3be77a50f064beda43bf11c5df6d4e97d
comment:9 Changed 10 months ago by
 Commit changed from c2e486c3be77a50f064beda43bf11c5df6d4e97d to e86ed66b7be556602c8173fafbdcc5cd45aba73e
Branch pushed to git repo; I updated commit sha1. New commits:
e86ed66  Trac #25219: add test case

comment:10 Changed 10 months ago by
 Description modified (diff)
 Status changed from needs_info to needs_review
Code updated as suggested.
I can't reproduce the bug anymore, either. I added it as a test case.
comment:11 followup: ↓ 12 Changed 10 months ago by
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.
comment:12 in reply to: ↑ 11 Changed 10 months ago by
Replying to vdelecroix:
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
andreverse_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.
comment:13 followup: ↓ 15 Changed 10 months ago by
 Status changed from needs_review to needs_info
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?
comment:14 Changed 10 months ago by
 Commit changed from e86ed66b7be556602c8173fafbdcc5cd45aba73e to ebd8f4500c2cdc09d701ffab9105fe72e9f4506d
Branch pushed to git repo; I updated commit sha1. New commits:
ebd8f45  Trac #25219: remove TEST that was already an EXAMPLE

comment:15 in reply to: ↑ 13 Changed 10 months ago by
Replying to vdelecroix:
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.
comment:16 Changed 10 months ago by
 Status changed from needs_info to positive_review
comment:17 Changed 10 months ago by
 Branch changed from u/ghBrentBaccala/25219 to ebd8f4500c2cdc09d701ffab9105fe72e9f4506d
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
Trac #25219: add reverse() method to LaurentSeries