Opened 5 years ago
Closed 5 years ago
#19532 closed enhancement (fixed)
asymptotic expansion generators related to singularity analysis
Reported by:  dkrenn  Owned by:  

Priority:  major  Milestone:  sage7.1 
Component:  asymptotic expansions  Keywords:  
Cc:  behackl, cheuberg  Merged in:  
Authors:  Daniel Krenn  Reviewers:  Clemens Heuberger 
Report Upstream:  N/A  Work issues:  
Branch:  a4eaf82 (Commits)  Commit:  a4eaf8251242fe1bd2bbfd4695e126cc01edcd3f 
Dependencies:  #19437, #19510, #19576, #19946, #19898  Stopgaps: 
Description (last modified by )
Create asymptotic expansions coming from pole singularities.
Change History (35)
comment:1 Changed 5 years ago by
 Branch set to u/dkrenn/asy/singularityanalysis
comment:2 Changed 5 years ago by
 Commit set to 3214c83c3a91f0eea73ad0068473661b1c228bfc
comment:3 Changed 5 years ago by
 Commit changed from 3214c83c3a91f0eea73ad0068473661b1c228bfc to 8ec2dfd1c61a30960d80cba160d46a93014cdad5
Branch pushed to git repo; I updated commit sha1. New commits:
8ec2dfd  fix docstring

comment:4 Changed 5 years ago by
 Branch changed from u/dkrenn/asy/singularityanalysis to u/cheuberg/asy/singularityanalysis
comment:5 Changed 5 years ago by
 Commit changed from 8ec2dfd1c61a30960d80cba160d46a93014cdad5 to ab1cd26496a64408e8701bfe306ee6ee399e2d76
comment:6 Changed 5 years ago by
 Commit changed from ab1cd26496a64408e8701bfe306ee6ee399e2d76 to 9796508e6aac3883a8175a19294456685ba37c64
Branch pushed to git repo; I updated commit sha1. New commits:
9796508  Trac #19532: fix doctest

comment:7 Changed 5 years ago by
 Milestone changed from sage6.10 to sage7.1
My impression is that you implemented poles and still want to implement logarithmic singularities in the same ticket. I'd suggest to restrict this ticket to poles (and raising NotImplementedError
for the logarithmic singularities, as apparently done in the current code) and to leave extensions to future tickets.
comment:8 Changed 5 years ago by
 Dependencies changed from #19437, #19510 to #19437, #19510, #19576
comment:9 Changed 5 years ago by
 Status changed from new to needs_review
comment:10 Changed 5 years ago by
 Commit changed from 9796508e6aac3883a8175a19294456685ba37c64 to a3a6dfd1d57038d14ee453d37884da19ad6d59cd
Branch pushed to git repo; I updated commit sha1. New commits:
f114f9e  Trac #19532: Add to table of contents

ede5173  Trac #19532: Minor documentation fixes

267718d  Trac #19532: Additional tests

0860173  Trac #19532: List log log factors, document missing implementations

a3a6dfd  Trac #19532: Add references

comment:11 followup: ↓ 17 Changed 5 years ago by
 Description modified (diff)
 Reviewers set to Clemens Heuberger
 Status changed from needs_review to needs_info
I reviewed the code, doctests pass, documentation and code are mostly fine. I added a few reviewer commits, please crossreview.
There is only the question on what to do with integral alpha <= 0
. Currently, the result is 0
which is certainly correct for sufficiently large values of the parameter. However, the rest of the asymptotics code as I remember it does not really assume "sufficiently large values of the parameter".
I am not really sure which result is appropriate. Several options:
0
 valid for sufficiently large values of the parameter. (current version)O(1)
 correct, but bad for applications. raise an error
 add a warning to the documentation
 add a note to the documentation
comment:12 Changed 5 years ago by
zeta != 1
is not implemented.
comment:13 Changed 5 years ago by
 Commit changed from a3a6dfd1d57038d14ee453d37884da19ad6d59cd to 7f74b313a5e309d3c2f2c1f3a280d7ad83418c4e
comment:14 Changed 5 years ago by
 Commit changed from 7f74b313a5e309d3c2f2c1f3a280d7ad83418c4e to f6a233d2c5563a86ec2adb320a42f92e18d62bae
comment:15 Changed 5 years ago by
 Branch changed from u/cheuberg/asy/singularityanalysis to u/cheuberg/asy/singularityanalysisgenerator
 Commit changed from f6a233d2c5563a86ec2adb320a42f92e18d62bae to 7f74b313a5e309d3c2f2c1f3a280d7ad83418c4e
pushed incorrect branch (mixup with #19944)
Last 10 new commits:
9796508  Trac #19532: fix doctest

f114f9e  Trac #19532: Add to table of contents

ede5173  Trac #19532: Minor documentation fixes

267718d  Trac #19532: Additional tests

0860173  Trac #19532: List log log factors, document missing implementations

a3a6dfd  Trac #19532: Add references

ea3dd35  small bugfix

1529a4d  Trac #19510: Merge #19306 (due to #19879)

b30388b  Trac #19532: Merge #19510 to fix doctest

7f74b31  Trac #19532: Implement zeta != 1

comment:16 Changed 5 years ago by
 Commit changed from 7f74b313a5e309d3c2f2c1f3a280d7ad83418c4e to f38cdfc7f03ff9c88d299080debd4700b9805538
Branch pushed to git repo; I updated commit sha1. New commits:
f38cdfc  Trac #19532: fix more issues with location of singularities

comment:17 in reply to: ↑ 11 Changed 5 years ago by
Replying to cheuberg:
There is only the question on what to do with integral
alpha <= 0
. Currently, the result is0
which is certainly correct for sufficiently large values of the parameter. However, the rest of the asymptotics code as I remember it does not really assume "sufficiently large values of the parameter".I am not really sure which result is appropriate. Several options:
0
 valid for sufficiently large values of the parameter. (current version)O(1)
 correct, but bad for applications. raise an error
 add a warning to the documentation
 add a note to the documentation
I think the result should be O(0)
. Currently I am thinking about an implementation of such a term and playing around a bit. If you want to get this ticket to positive, then I would raise a NotImplementedError? at the moment. But give me a couple of days to figure out how O(0)
would be possible.
Best, Daniel
comment:18 followup: ↓ 19 Changed 5 years ago by
What would O(0)
mean? How would that affect #19944?
comment:19 in reply to: ↑ 18 ; followup: ↓ 20 Changed 5 years ago by
Replying to cheuberg:
What would
O(0)
mean?
f(n)
is in O(0)
if there is an N
such that f(n)=0
for n >= N
. Thus the coefficients of polynomials (seen as power series) would be in O(0)
.
How would that affect #19944?
E.g. inserting the generating function (1z)^alpha
for nonnegative integers alpha
would result in a O(0)
, but this behavior comes from this ticket here. I am not sure if #19944 would need adaptions.
comment:20 in reply to: ↑ 19 ; followup: ↓ 21 Changed 5 years ago by
Replying to dkrenn:
Replying to cheuberg:
What would
O(0)
mean?
f(n)
is inO(0)
if there is anN
such thatf(n)=0
forn >= N
. Thus the coefficients of polynomials (seen as power series) would be inO(0)
.
so O(0)
would be smaller than any other element in the poset and be absorbed by any other error term? Would it live in some kind of n^ZZbar
with ZZbar = { infinity} \cup ZZ
?
How would that affect #19944?
E.g. inserting the generating function
(1z)^alpha
for nonnegative integersalpha
would result in aO(0)
, but this behavior comes from this ticket here. I am not sure if #19944 would need adaptions.
ok, may be.
The question is whether it is worth the effort. After working on #19944 I doubt that this generator here will frequently be called outside of #19944. After all, calling #19944 with a polynomial as an argument is erroneous anyway, because polynomials do not have singularities. Thus we should never have to return O(0)
so we might as well go for a warning box here and be done with the problem.
comment:21 in reply to: ↑ 20 Changed 5 years ago by
Replying to cheuberg:
Replying to dkrenn:
f(n)
is inO(0)
if there is anN
such thatf(n)=0
forn >= N
. Thus the coefficients of polynomials (seen as power series) would be inO(0)
.so
O(0)
would be smaller than any other element in the poset and be absorbed by any other error term? Would it live in some kind ofn^ZZbar
withZZbar = { infinity} \cup ZZ
?
I'm afraid, we would really need a growth of 0
to make it work correctly with cartesian products.
E.g. inserting the generating function
(1z)^alpha
for nonnegative integersalpha
would result in aO(0)
, but this behavior comes from this ticket here. I am not sure if #19944 would need adaptions.ok, may be.
The question is whether it is worth the effort. After working on #19944 I doubt that this generator here will frequently be called outside of #19944. After all, calling #19944 with a polynomial as an argument is erroneous anyway, because polynomials do not have singularities. Thus we should never have to return
O(0)
so we might as well go for a warning box here and be done with the problem.
I am raising a special NotImplementedError
now. Code follows...
comment:22 Changed 5 years ago by
 Branch changed from u/cheuberg/asy/singularityanalysisgenerator to u/dkrenn/asy/singularityanalysisgenerator
comment:23 Changed 5 years ago by
 Commit changed from f38cdfc7f03ff9c88d299080debd4700b9805538 to 2dff0518eb6644590963ffe56bb6122c5c160579
 Status changed from needs_info to needs_review
I've crossreview your changes...seem to be fine.
New commits:
2dff051  raise NotImplementedOZero instead of the wrong result 0

comment:24 Changed 5 years ago by
 Status changed from needs_review to positive_review
LGTM. Setting to positive despite the dependency: the dependency is only in order to avoid a merge conflict (inserting new methods at the same position); so no changes are necessary here anyway independently of the fate of #19510.
comment:25 Changed 5 years ago by
 Branch changed from u/dkrenn/asy/singularityanalysisgenerator to u/cheuberg/asy/singularityanalysisgenerator
comment:26 Changed 5 years ago by
 Commit changed from 2dff0518eb6644590963ffe56bb6122c5c160579 to 21054cb615b46f4c857a9c247be638baf10c663b
 Dependencies changed from #19437, #19510, #19576 to #19437, #19510, #19576, #19946
 Status changed from positive_review to needs_review
Merged #19946 and removed workaround for #19946; please crossreview.
New commits:
1fc00c7  Trac #19946: fix _pushout_ for cartesian product of growth groups

1b62954  Trac #19946: add doctests to document behavior

24c153c  Trac #19946: reviewer commit: ReSt error

7918417  Trac #19946: additional doctest

bfb72dc  Trac #19532: Merge #19946

21054cb  Trac #19532: get rid of workaround for #19946

comment:27 followup: ↓ 28 Changed 5 years ago by
 Branch changed from u/cheuberg/asy/singularityanalysisgenerator to u/dkrenn/asy/singularityanalysisgenerator
comment:28 in reply to: ↑ 27 Changed 5 years ago by
 Commit changed from 21054cb615b46f4c857a9c247be638baf10c663b to a71b84a84dee6c3ee8fb4957f5f606dec8ed96b0
I've merged in the dependency #19510 due to changes there and mergeconflicts here.
Last 10 new commits:
5d9152c  Trac #19510: state constant factors/summands in Stirling and binomial

2db9105  Trac #19510: separate calculation of negative powers in log_Stirling

1471336  Trac #19946: rewrite and improve explaination of 1b62954

9f9a23f  Trac #19961: document rpow

101e4de  Merge branch 'u/dkrenn/t/19961' of trac.sagemath.org:sage into t/19946

7179c78  Trac #19946: link from general doc to detailed explaination

1259201  Trac #19946: fix typo

37caf25  Merge branch 'u/cheuberg/t/19946' of trac.sagemath.org:sage into t/19510/asy/generatorsbinomial

10cd04b  Trac #19510: major rewrite of binomial kn over n

a71b84a  Merge remotetracking branch 'trac/u/dkrenn/asy/generatorsbinomial' into t/19532/asy/singularityanalysisgenerator

comment:29 Changed 5 years ago by
 Commit changed from a71b84a84dee6c3ee8fb4957f5f606dec8ed96b0 to 4ad4524df848ba9ddc3dace2d22b993399f6feb1
Branch pushed to git repo; I updated commit sha1. New commits:
4c58608  Trac #19898: Generator for expansion of harmonic number

adae74b  Trac #19898: Merge #19510 to resolve conflicts

ef9cb08  Trac #19898: Merge #19306 (due to #19879)

e193ac1  Trac #19898: sage.rings.arith > sage.arith.all (after #19879)

bedd23a  Trac #19898: rename harmonic_number to HarmonicNumber

27e9e69  Trac #19898: state constant summand in documentation

209329d  Merge branch 'u/dkrenn/asy/generatorsbinomial' of trac.sagemath.org:sage into t/19898/asy/harmonicnumber

4ad4524  Merge branch 'u/dkrenn/asy/harmonicnumber' of trac.sagemath.org:sage into t/19532/asy/singularityanalysisgenerator

comment:30 Changed 5 years ago by
 Dependencies changed from #19437, #19510, #19576, #19946 to #19437, #19510, #19576, #19946, #19898
comment:31 Changed 5 years ago by
 Branch changed from u/dkrenn/asy/singularityanalysisgenerator to u/cheuberg/asy/singularityanalysisgenerator
comment:32 Changed 5 years ago by
 Commit changed from 4ad4524df848ba9ddc3dace2d22b993399f6feb1 to 2b95429752c5967ca69422a219a45c534e905824
Merged #19898 once more and added a doctest; please review this new doctest and then set to positive if satisfied.
Last 10 new commits:
3d51289  Trac #19957: Merge #19946 to avoid workaround

b606fdf  Trac #19957: remove workaround for #19946

4346994  Trac #19957: rename e to expansion in doctests

5d6e9b2  Trac #19957: Introduce parameter relative tolerance

00eae5d  Merge branch 't/19957/asy/comparewithvalues' into t/19510/asy/generatorsbinomial

78bdd0f  Trac #19510: additional doctests using compare_with_values

148eabb  Trac #19898: Merge #19510

a66c377  Trac #19898: additional doctest using compare_with_values

5dc6f58  Trac #19532: Merge #19898

2b95429  Trac #19532: additional doctest using compare_with_values

comment:33 Changed 5 years ago by
 Commit changed from 2b95429752c5967ca69422a219a45c534e905824 to a4eaf8251242fe1bd2bbfd4695e126cc01edcd3f
Branch pushed to git repo; I updated commit sha1. New commits:
a4eaf82  Trac #19532: add relative tolerance to doctests introduced on this ticket

comment:34 Changed 5 years ago by
 Status changed from needs_review to positive_review
comment:35 Changed 5 years ago by
 Branch changed from u/cheuberg/asy/singularityanalysisgenerator to a4eaf8251242fe1bd2bbfd4695e126cc01edcd3f
 Resolution set to fixed
 Status changed from positive_review to closed
Implemented pole singularities.
Last 10 new commits:
simplify coefficients automatically and use the faster algorithm per default
docu
result over QQ if skipparameter given
toc entry (at top of file)
SR.symbol: set parent correctly (inheritance)
Move pynac_symbol_registry to cdef attribute SR.symbols
Merge branch 'u/jdemeyer/symbolic/subvar' of trac.sagemath.org:sage into asy/singularityanalysis
calculate coefficients of singularity analysis (pole type)
generator SingularityAnalysis (pole type)
explicitly specify a default defaultprecision