Opened 4 years ago

Closed 4 years ago

#20053 closed enhancement (fixed)

Singularity analysis for given singular expansions

Reported by: cheuberg Owned by:
Priority: minor Milestone: sage-7.1
Component: asymptotic expansions Keywords: singularity analysis
Cc: dkrenn, behackl Merged in:
Authors: Clemens Heuberger Reviewers: Benjamin Hackl
Report Upstream: N/A Work issues:
Branch: b42bb05 (Commits) Commit: b42bb05ee54ac7f70df77470b4c4c81a1631f0b2
Dependencies: #20056, #19540 Stopgaps:

Description (last modified by cheuberg)

Refactor coefficients_of_generating_function (#20056) so that the part working with one singular expansion is now a separate method of an asymptotic expansion.

This allows to perform singularity analysis also in situations where the singular expansion has been obtained by other means than the simple approach for explicit generating functions of #19944 (and follow-ups #20040 and #20056), even while #20050 is not implemented.

For now, this is a hidden method because the input will change once #20050 is done.

Once #20050 is implemented, the dual choice of output of coefficients_of_generating_function may be changed: if users want to have singular expansions (in order to show them in their paper, for instance), they could convert it into a singular expansion by using #20050 (or its dependents) and then feed it into singularity_analysis.

See #17601 for the asymptotic expansions meta ticket.

Change History (22)

comment:1 Changed 4 years ago by cheuberg

  • Branch set to u/cheuberg/asy/allow-singular-expansion

comment:2 follow-up: Changed 4 years ago by cheuberg

  • Authors set to Clemens Heuberger
  • Commit set to 8bb904679aae25cb490a735fb492e43a672d356b
  • Status changed from new to needs_review

That is a bare bone implementation of what I want; I set it to needs_review because I'd like to read your opinions.


Last 10 new commits:

9ce7b35Trac #19944: exchange order of parameters var and zeta in _singularity_analysis_
902c182Trac #20040: Merge #19944
866a9e3Trac #20040: exchange order of parameters var and zeta in _singularity_analysis_
c79a6f7Trac #19944: exchange zeta and var in docstrings
d0f5e94Trac #20040: Merge #19944
27a8605Merge branch 'u/cheuberg/asy/singularity-analysis-method-log' of git://trac.sagemath.org/sage into t/20040/asy/singularity-analysis-method-log
452c43bTrac #19944: _singularity_analysis_ methods: more precise documentation
4cb5934Trac #20040: Merge #19944
8455dc2Trac #20040: _singularity_analysis_ methods: more precise documentation
8bb9046Trac #20040: added myself to AUTHORS

comment:3 Changed 4 years ago by cheuberg

  • Dependencies changed from #20052, #19944, #20050 to #20052, #20040, #20050

comment:4 Changed 4 years ago by git

  • Commit changed from 8bb904679aae25cb490a735fb492e43a672d356b to 271f1323b262f50ca1bfc0941ab49815bd517af7

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

271f132Trac #20053: allow singular expansions as input of singularity analysis

comment:5 in reply to: ↑ 2 ; follow-up: Changed 4 years ago by dkrenn

Replying to cheuberg:

That is a bare bone implementation of what I want; I set it to needs_review because I'd like to read your opinions.

This extensions does not work with more than one singularity as the same singular expansion is used for each singularity.

comment:6 in reply to: ↑ 5 Changed 4 years ago by cheuberg

Replying to dkrenn:

Replying to cheuberg:

That is a bare bone implementation of what I want; I set it to needs_review because I'd like to read your opinions.

This extensions does not work with more than one singularity as the same singular expansion is used for each singularity.

  • Option 1: forbid more than one singularity
  • Option 2: function could be a list of singular expansions
  • Option 3: function could be a dictionary, as it is the case for the output when selected.

I do not like option 3, as it is not relevant once #20050 is implemented.

comment:7 Changed 4 years ago by cheuberg

A completely different option would be to have singularity_analysis as a method of an AsymptoticExpansion and to call singular_expansion.singularity_analysis('n', precision=5). However, we would then have AsymptoticRing.singularity_analysis which ends up in that ring and AsymptoticExpansion.singularity_analysis which would end up somewhere else.

comment:8 follow-up: Changed 4 years ago by cheuberg

Proposal: rename AsymptoticRing.singularity_analysis to AsymptoticRing.coefficients_of_generating_function and have AsymptoticExpansion._singularity_analysis_ and make this method public once #20050 is done.

comment:9 in reply to: ↑ 8 Changed 4 years ago by dkrenn

Replying to cheuberg:

Proposal: rename AsymptoticRing.singularity_analysis to AsymptoticRing.coefficients_of_generating_function and have AsymptoticExpansion._singularity_analysis_ and make this method public once #20050 is done.

+1

comment:10 Changed 4 years ago by git

  • Commit changed from 271f1323b262f50ca1bfc0941ab49815bd517af7 to 7f8c745479c0a5fd7372f5bd9c6c05c5a644f648

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

40b6e2eTrac #20056: Rename AsymptoticRing.singularity_analysis to coefficients_generating_function
197fe43Trac #20053: Merge #20056
bbbfab9Trac #20053: Also rename method here
071a595Trac #20053: Refactor: new method AsymptoticRing._singularity_analysis_
2b2e94dTrac #20053: minor cleanup in coefficients_of_generating_function
7f8c745Trac #20053: add warning to coefficients_of_generating_function

comment:11 Changed 4 years ago by cheuberg

  • Dependencies changed from #20052, #20040, #20050 to #20056
  • Description modified (diff)

This is now a complete rewrite following yesterday's discussions and #20056.

I now think that this is basically the point that we can reach without #20050, so this now awaits your review.

comment:12 Changed 4 years ago by cheuberg

  • Summary changed from Singularity analysis: allow singular expansions as input to Singularity analysis for given singular expansions

comment:13 Changed 4 years ago by git

  • Commit changed from 7f8c745479c0a5fd7372f5bd9c6c05c5a644f648 to 7acc4879e8992b286119e0ac6c0caf365032b9e5

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

8a43e7eTrac #19540: allow growth groups strings in TermMonoid factory
0384e25Trac #19540: use is_one()
3f0f855Trac #19540: variable_names for growth elements
eb49c65Trac #19540: variable_names for terms
5a4adf0Trac #19540: variable_names for asymptotic expansions
d9f2796Trac #19540: _factorial_ for terms
c801853Trac #19540: rewrite factorial
139f7c8Trac #19540: note on log(s).factorial()
23948e4Merge branch 'u/cheuberg/asy/singularity-analysis-method' of git://trac.sagemath.org/sage into t/19540/asy/factorial
7acc487Trac #20053: Merge branch #19540 to resolve merge conflict

comment:14 Changed 4 years ago by cheuberg

  • Dependencies changed from #20056 to #20056, #19540

comment:15 follow-up: Changed 4 years ago by behackl

I've reviewed this implementation, and I only have one comment:

Could you revise the docstring of AsymptoticExpansion._singularity_analysis_? Because

  1. var can also be the generator of an asymptotic ring,
  2. zeta is the location of the singularity, and
  3. precision is not set to the default precision of the ring if it is omitted, but has the same behavior as the SingularityAnalysis method in the generators.

Do we want that the current behavior from 3 happens, so that the asymptotic contribution has the same precision as the singular expansion by default? Would be worth considering IMHO.

Other than that, positive_review form my side.

comment:16 Changed 4 years ago by git

  • Commit changed from 7acc4879e8992b286119e0ac6c0caf365032b9e5 to b42bb05ee54ac7f70df77470b4c4c81a1631f0b2

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

713f05fTrac #20053: generator of asymptotic ring can be used
b42bb05Trac #20053: modify behaviour of precision

comment:17 in reply to: ↑ 15 ; follow-up: Changed 4 years ago by cheuberg

Replying to behackl:

  1. var can also be the generator of an asymptotic ring,

documented and doctested

  1. zeta is the location of the singularity, and

done.

  1. precision is not set to the default precision of the ring if it is omitted, but has the same behavior as the SingularityAnalysis method in the generators.

Do we want that the current behavior from 3 happens, so that the asymptotic contribution has the same precision as the singular expansion by default? Would be worth considering IMHO.

We cannot directly use the precision of this element, because we have no elegant means of defining the precision of an element (we will not introspect the group). Thus IMHO the only sensible default is the default precision of the parent of this element. Thus the behaviour is now changed.

The method coefficients_of_generating_function does not handle the precision, either, but that could be another ticket, if you want to open one. In any case, this is unrelated to this method here.

comment:18 in reply to: ↑ 17 Changed 4 years ago by behackl

Replying to cheuberg:

Replying to behackl:

  1. var can also be the generator of an asymptotic ring,

documented and doctested

  1. zeta is the location of the singularity, and

done.

  1. precision is not set to the default precision of the ring if it is omitted, but has the same behavior as the SingularityAnalysis method in the generators.

Do we want that the current behavior from 3 happens, so that the asymptotic contribution has the same precision as the singular expansion by default? Would be worth considering IMHO.

We cannot directly use the precision of this element, because we have no elegant means of defining the precision of an element (we will not introspect the group). Thus IMHO the only sensible default is the default precision of the parent of this element. Thus the behaviour is now changed.

That is what I originally meant, thank you.

The method coefficients_of_generating_function does not handle the precision, either, but that could be another ticket, if you want to open one. In any case, this is unrelated to this method here.

Yes, this would certainly be a new ticket, and I think that this is definitely worth opening one.

In any case: your changes look good to me, and I'll just wait for the patchbot before I set this to positive_review. Thank you!

comment:19 Changed 4 years ago by behackl

  • Status changed from needs_review to positive_review

The patchbot is happy, and so am I. :-)

comment:20 Changed 4 years ago by vbraun

  • Status changed from positive_review to needs_work

Reviewer name is missing

comment:21 Changed 4 years ago by cheuberg

  • Reviewers set to Benjamin Hackl
  • Status changed from needs_work to positive_review

comment:22 Changed 4 years ago by vbraun

  • Branch changed from u/cheuberg/asy/allow-singular-expansion to b42bb05ee54ac7f70df77470b4c4c81a1631f0b2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.