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

Singularity analysis for given singular expansions #20053

Closed
cheuberg opened this issue Feb 14, 2016 · 29 comments
Closed

Singularity analysis for given singular expansions #20053

cheuberg opened this issue Feb 14, 2016 · 29 comments

Comments

@cheuberg
Copy link
Contributor

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.

Depends on #20056
Depends on #19540

CC: @dkrenn @behackl

Component: asymptotic expansions

Keywords: singularity analysis

Author: Clemens Heuberger

Branch/Commit: b42bb05

Reviewer: Benjamin Hackl

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

@cheuberg
Copy link
Contributor Author

@cheuberg
Copy link
Contributor Author

comment:2

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

@cheuberg
Copy link
Contributor Author

Commit: 8bb9046

@cheuberg
Copy link
Contributor Author

Author: Clemens Heuberger

@cheuberg
Copy link
Contributor Author

Changed dependencies from #20052, #19944, #20050 to #20052, #20040, #20050

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 15, 2016

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

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 15, 2016

Changed commit from 8bb9046 to 271f132

@dkrenn
Copy link
Contributor

dkrenn commented Feb 15, 2016

comment:5

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.

@cheuberg
Copy link
Contributor Author

comment:6

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.

@cheuberg
Copy link
Contributor Author

comment:7

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.

@cheuberg
Copy link
Contributor Author

comment:8

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.

@dkrenn
Copy link
Contributor

dkrenn commented Feb 15, 2016

comment:9

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 16, 2016

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 16, 2016

Changed commit from 271f132 to 7f8c745

@cheuberg
Copy link
Contributor Author

Changed dependencies from #20052, #20040, #20050 to #20056

@cheuberg

This comment has been minimized.

@cheuberg
Copy link
Contributor Author

comment:11

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.

@cheuberg cheuberg changed the title Singularity analysis: allow singular expansions as input Singularity analysis for given singular expansions Feb 16, 2016
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 16, 2016

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 16, 2016

Changed commit from 7f8c745 to 7acc487

@cheuberg
Copy link
Contributor Author

Changed dependencies from #20056 to #20056, #19540

@behackl
Copy link
Member

behackl commented Mar 8, 2016

comment:15

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.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 9, 2016

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 9, 2016

Changed commit from 7acc487 to b42bb05

@cheuberg
Copy link
Contributor Author

cheuberg commented Mar 9, 2016

comment:17

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.

@behackl
Copy link
Member

behackl commented Mar 9, 2016

comment:18

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!

@behackl
Copy link
Member

behackl commented Mar 9, 2016

comment:19

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

@vbraun
Copy link
Member

vbraun commented Mar 9, 2016

comment:20

Reviewer name is missing

@cheuberg
Copy link
Contributor Author

Reviewer: Benjamin Hackl

@vbraun
Copy link
Member

vbraun commented Mar 10, 2016

Changed branch from u/cheuberg/asy/allow-singular-expansion to b42bb05

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