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

AsymptoticRing: exp & log, cleanup, some improvements, documentation #19083

Closed
dkrenn opened this issue Aug 25, 2015 · 162 comments
Closed

AsymptoticRing: exp & log, cleanup, some improvements, documentation #19083

dkrenn opened this issue Aug 25, 2015 · 162 comments

Comments

@dkrenn
Copy link
Contributor

dkrenn commented Aug 25, 2015

Logarithm and exponential function (both with arbitrary appropriate bases) of asymptotic expressions.

Cleanup, some improvements, documentation in the asymptotic ring framework (see also #17601).

Depends on #19073

CC: @behackl @cheuberg

Component: asymptotic expansions

Author: Benjamin Hackl, Daniel Krenn

Branch/Commit: f9fbccb

Reviewer: Daniel Krenn, Clemens Heuberger

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

@dkrenn dkrenn added this to the sage-6.9 milestone Aug 25, 2015
@dkrenn
Copy link
Contributor Author

dkrenn commented Aug 25, 2015

Branch: u/dkrenn/asy/prototype

@dkrenn
Copy link
Contributor Author

dkrenn commented Aug 25, 2015

@dkrenn
Copy link
Contributor Author

dkrenn commented Aug 25, 2015

Commit: e9acd0e

@dkrenn
Copy link
Contributor Author

dkrenn commented Aug 25, 2015

Last 10 new commits:

a23d36econstruction and functor and doctests
3b49e70pushout: correct str/repr (switch to be constistent with remaining Sage)
a3b2cc6Merge branch 'pushout/functor-str' into u/dakrenn/asy/rings-coercion
19247a5fix derived classes
dacbfcbmore replacements of `__str__` by _repr_
d1356e5Merge branch 'pushout/functor-str' into u/dakrenn/asy/rings-coercion
27601bafix missing docs
e1acc40improve docs (seealso, links, etc)
98e8860make doctest coverage 100% (asymptotic_ring)
e9acd0ecleanup (remove a comment)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 25, 2015

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

9c1fc0ecleanup documentation: strange 'exact removed
ddbd90acollect asymptotic code in one directory
ae47bcbMerge remote-tracking branch 'origin/u/behackl/asy/asymptoticExpression' into t/19048/asy/an_element
3a44a01fix imports to make doctests pass
171d232Merge branch 'u/dkrenn/asy/an_element' of trac.sagemath.org:sage into asy/prototype
702527eMerge commit '695f08d2dd5d98589dfc10b3f2da89ce4e7dd908' of trac.sagemath.org:sage into asy/prototype
3cacc52change recursive pushout to common_parent (to take care of direct coercions)
4662eedbug kind of solved (called now directly by cartesian_product); move doctest
23354dbadd missing test and fix broken tests
078d327Merge remote-tracking branch 'origin/u/dkrenn/18182-on-6.7' into asy/prototype

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 25, 2015

Changed commit from e9acd0e to 078d327

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 26, 2015

Changed commit from 078d327 to 97cdf16

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 26, 2015

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

263598csome doc fixes in term_moniod
a382656add a link in growth_group_factory
c549344some improvments in the module description of the asymptotic ring
45549cdbig_oh: some seealso
0bf9781fix doctests
223c1e3move parts in documentation-tree
adc9236write doc on used data structures
42a5c1etwo more headlines
d1f4611fix docbuild (wrong apostrophe)
97cdf16small change in division example

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 26, 2015

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

d759c8eallow O(0)
ab05e75extend error messages in element_constructor and test for nonzeroness
4bb85a5rewrite element_constructor of terms
da0a68aterm: rename base_ring to coefficient_ring
ffe1db8make it working again and fix doctests
ff7fb30change `_repr_` of monoids
29ed6b3move element_construction to GenericTermMonoid
1ac20d7rewrite element_constructor of AsymptoticRing to accept SR and polynomials
15ba65ecorrect a bug in growth_group_cartesian._element_constructor_
4bbe5f3add a doctest to convert from SR

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 26, 2015

Changed commit from 97cdf16 to 4bbe5f3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 26, 2015

Changed commit from 4bbe5f3 to 8e0428e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 26, 2015

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

c958237docs and doctests for 100%
b8ebe81' * ' to '*' in elements
8e0428eremove special treating of 1/x in `_repr_` (due to inconsitency)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 27, 2015

Changed commit from 8e0428e to d59304b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 27, 2015

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

4115f61add a pushout-doctest
ad1ca3fextend and simplify element_constructor and use convert-flag everywhere
e05a7fbcategories and __classcall__
38075d0__invert__: take care of changing parent
6d1f0c3small changes in multiplication code
66948e2rmul, lmul
f48f64adoc
aad4bf7convert NN in growth group factory doctest
b2753edchange _an_element_: remove - to avoid infinite loop in get_action
d59304bmake AsymptoticRing derived from Algebra

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 28, 2015

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

57cd39fdoc of rmul
1e625e3add a doctest for scalar multiplication
be23e63fix bug with "is" and change_parameter
0c2b6e4improves working with categories and use CommutativeAlgebras()
ee071a7fix scalar multiplication by zero
f54e588some checks on the given names of the generators (when using R.<....>)
92861edmore on names/gens
18bc9e8correct handling of parent in __invert__
d833544minor rewrite of __pow__

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 28, 2015

Changed commit from d59304b to d833544

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 28, 2015

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

caef9eaterm: is_same --> __eq__
5618385rewrite has_same_summands to use iterators efficiently
51cfce6adapt doc
ef4160eMerge branch 'asy/inversion' into asy/prototype
e4bf138fix failing doctest after merge

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 28, 2015

Changed commit from d833544 to e4bf138

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 28, 2015

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

a13f6cbwrite map and mapped
6b747e6mapping argument for .copy()
edfd1b8bring map and mapped back to work and use new copy-construction
4a33e4ffix bug in merge
7680531make truncate faster
c7c02b0element.__init__: check if summands are of correct type
4659972rewrite `__init__` of element to use new feature of MutablePoset
7287ca1make `_mul_term_` faster

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 28, 2015

Changed commit from e4bf138 to 7287ca1

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 28, 2015

Changed commit from 7287ca1 to 6c04529

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 28, 2015

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

2863321allow merge to go over all elements
9391c78derive MutablePoset from object (instead of SageObject)
70859d7cleanup: delete toset since not yet implemented
d647d95write map and mapped
e64cbd8mapping argument for .copy()
c28749cbring map and mapped back to work and use new copy-construction
ceb0b37fix bug in merge
123d0c8Merge branch 'u/dkrenn/asy/mutable-poset' into asy/prototype
6c04529fix lazy import problem when using short representations

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 28, 2015

Changed commit from 6c04529 to 5068df9

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 28, 2015

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

1d023c6import AsymptoticRing lazy
e02895fremove some empty lines
738f4edexplicitly use coefficient_ring(-1) in subtraction
ecf5ba8use simplify/convert everywhere explicit
55d6ad3restructure and rewrite classcall and init of growth group
3e449dfsolve non-uniquenessbug by rewriting classcall and init of term
5068df9rewrite classcall and init of asymptotic ring

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 30, 2015

Changed commit from 5068df9 to 93b8e0b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 30, 2015

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

25231e1rewrite `__pow__` of asymptotic ring
5408f6dAsymptotic Term with ... --> Term with ...
8730ab8creation of element via parent method for ring
ac3d1ecadapt __pow__
fff27dafix FloatingPointException (vs. ZeroDivisonError)
1097deecreate misc.py and move functions from other files to this file
8bae4a9change imports to make everything running again
184ead8use relative imports where possible
ed536a1use asymptotic_ring option when calling factory TermMonoid
93b8e0bcorrect thing reported by pyflakes

@dkrenn
Copy link
Contributor Author

dkrenn commented Oct 16, 2015

comment:86

Replying to @cheuberg:

  1. AsymptoticExpansion.__init__:
    • parameter convert is neither documented nor tested.

Now it is.

  • test error "Cannot include ..."

Doctest added.

  1. AsymptoticExpansion._mul_term_: I'd prefer to say simplify = not isinstance(term, ExactTerm) because this seems to be more robust with respect to future extensions.

True, good idea; changed.

  1. AsymptoticExpansion.__invert__:
    • test errors

Done.

@dkrenn
Copy link
Contributor Author

dkrenn commented Oct 16, 2015

comment:87
  1. AsymptoticExpansion.__invert__:
    • I think that there is shared code between __invert__, __log__, __exp__ and powers with rational exponents. In all those cases, it is important to split into main term and renormalized remainder. The main term is then processed according to the respective method, the remainder is inserted into a converging taylor series with certain coefficients (this could be handeled by one method getting the sequence of taylor coefficients as an argument). Possibly in a follow-up ticket.

Yes, I've noticed this as well. I have it on my local todo list. Now this is #19423

@dkrenn
Copy link
Contributor Author

dkrenn commented Oct 16, 2015

comment:88
  1. AsymptoticExpansion.__pow__:
    • why is (y^2 + O(y))^(1/2) not tested (add a comment?)

I've refered to #19316.

  • test error: "Cannot take ... to the negative exponent"
  • test error: "Taking ... to the exponent ... not implemented"

Added tests.

  • rational exponents could be implemented via binomial series (follow-up ticket?)

A ticket (#19316) already exists.

  1. AsymptoticExpansion.log:
    • test error ("several maximal elements")

Added.

  • I am not convinced that having k as an AsymptoticExpansion is very efficient (let the division be in QQ and convert later?, cf. rpow.)

Changed and made some tests: I could save about 15% cputime :)

Rest will follow later today.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 16, 2015

Changed commit from 44b52d6 to 8006837

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 16, 2015

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

2522bd0Trac #19094/#19083 comment 66, 36: add error test
8cbacdfTrac #19094/#19083 comment 66, 37: refer to #19424 in not tested doctest
ec10395Trac #19094/#19083 comment 66, 39: extend description of old_parent (in
36a48e4Trac #19094/#19083 comment 66, 39: `_create_element_in_extension_` rewrite doctests and rename parameter to old_term_parent
38a597bTrac #19094/#19083 comment 66, 40: remove outdated NOTE block
b927671Trac #19094/#19083 comment 66, 40: complete doctests of AsymptoticRing._element_constructor_
7624627Trac #19094/#19083 comment 66, 40: refer to trac tickets at O-Term from SR todo
1ceba10Trac #19094/#19083 comment 66, 40: test conversion from multivariate polynomial ring
8006837Trac #19094/#19083 comment 66, 40: simplify test for empty data

@dkrenn
Copy link
Contributor Author

dkrenn commented Oct 16, 2015

comment:90

Replying to @cheuberg:

  1. AsymptoticExpansion.rpow:
    • Decrease indentation of ALGORITHM block

Done.

  • Test error "Cannot construct the power ..."

Added test.

  1. AsymptoticRing: Add some comment on the "TESTS" section which is currently inactive.

Added (#19424).

  1. AsymptoticRing.change_parameter: is this a good name (the method sounds like an inplace change, but usually returns a new parent instead)

True, but consistent with

sage: R.<z> = ZZ[]
sage: R.change_ring(QQ)
Univariate Polynomial Ring in z over Rational Field
sage: R
Univariate Polynomial Ring in z over Integer Ring
  1. AsymptoticRing._create_element_via_parent_:
    • I do not like the name and the description of the method.

Changed earlier already.

  • The role of old_parent is not documented

Extended doc.

  • In all uses of the method in this module, old_parent is set and is a term monoid. In the example of this method, it is an asymptotic ring.

Uuups...changed.

  • I do not understand why old_parent is needed: if term.parent() is old_parent, then self.change_parameter will probably not change the parent anyway.

E.g. when inverting, then the term parameter will be the already inverted term and old_parent the parent of the original term.

  1. AsymptoticRing._element_constructor_:
    • The note box seems to be outdated, as the parameter summands no longer exists.

Deleted.

  • test error "Not all list entries ..."
  • test error "Symbolic expression ... is not in ..."

Added.

  • Why not data or data == 0 instead of not data?

Changed.

  • What happens with an O-Term in SR?

There seems to be a bug in SR: #19425. Opened follow up #19426.

  • conversion of multivariate polynomial ring is not tested.

Now it is.

  1. Delete the following in src/doc/en/reference/rings/index.rst
.. toctree::
   :hidden:

   asymptotic_expansions_index

Not possible, since in src/doc/en/reference/index.rst all indices are included
via :doc: which seems not to add them to any toctree.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 16, 2015

Changed commit from 8006837 to a848139

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 16, 2015

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

4e5af11Trac #19094/#19083 comment 66, 36: delete ALGORITHM block
cb083d4Trac #19094/#19083 comment 66, 41: delete `_create_exact_summands_` since not needed
cf228bbTrac #19094/#19083 comment 66, 42: make creating of exact summand with growth but without coefficient impossible
a848139Trac #19094/#19083 comment 66, 42: create_summands: add doctests for interesting `data`

@dkrenn
Copy link
Contributor Author

dkrenn commented Oct 16, 2015

comment:92

Replying to @cheuberg:

  1. AsymptoticExpansion.rpow:
    • The algorithm given in the documentation seems to be quite obvious
    • I do not like "the rest", "the remaining part": those are actually the asymptotic main parts.
    • Why is the algorithm described in that much detail here and nowhere (log, exp, pow) else?

Block deleted.

  1. AsymptoticRing._element_constructor_:
    • I do not understand the sentence "If set, then it is assured that the terms belong to this asymptotic ring (by converting them if needed)." Who does the converting?

Meanwhile the convert parameter in AsymptoticExpansion has a better description.

  1. AsymptoticRing._create_exact_summand_: Why is this needed and not handeled by .create_summand?

Deleted and remaining code adapted.

  1. AsymptoticRing.create_summand:
    • doctest case of interesting data (i.e. with coefficient and growth element).

Added.

  • The following is surprising:
sage: AR.<z> = AsymptoticRing('z^QQ', QQ)
sage: AR.create_summand('exact', growth='z^2')
z^2
I'd rather expect an error or zero than using an implicit coefficient of `1`

Changed; now there is an error.

@cheuberg
Copy link
Contributor

Changed branch from u/dkrenn/asy/prototype to u/cheuberg/asy/prototype

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 17, 2015

Changed commit from a848139 to f9fbccb

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 17, 2015

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

de137d1Trac #19083: fix one ReSt error
f9fbccbTrac #19083: expand description of parameter "convert"

@cheuberg
Copy link
Contributor

comment:95

I checked the changes again, added two trivial commits. Doctests pass, documentation builds, trac's automerge fails for unknown reasons, but branch merges cleanly into 6.10.beta0.

@vbraun
Copy link
Member

vbraun commented Oct 24, 2015

Changed branch from u/cheuberg/asy/prototype to f9fbccb

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