Opened 4 years ago

Closed 4 years ago

#19048 closed enhancement (fixed)

AsymptoticRing: an_element

Reported by: dkrenn Owned by:
Priority: major Milestone: sage-6.9
Component: asymptotic expansions Keywords: asymptotics
Cc: behackl Merged in:
Authors: Daniel Krenn Reviewers: Benjamin Hackl, Clemens Heuberger
Report Upstream: N/A Work issues:
Branch: 617c593 (Commits) Commit: 617c5933907d132785ee1052473dfa808350ec04
Dependencies: #17716, #19047, #19068, #19319 Stopgaps:

Description (last modified by dkrenn)

Implement .an_element and .some_elements.

See also meta-ticket #17601.

Change History (29)

comment:1 Changed 4 years ago by dkrenn

  • Branch set to u/dkrenn/asy/an_element

comment:2 Changed 4 years ago by dkrenn

  • Authors set to Daniel Krenn
  • Commit set to 4f294ce9023fc4908bb3ad67228f900111826158

Last 10 new commits:

fbd6c93implement QQ.some_elements()
c486e99Merge branch 'rings/QQ-some-elements' into asy/an_element
0a13882improve doctests of _an_element_
0f7412asome_elements for growth groups
57d1720product_diagonal iterator
fc20bf8improve doctest of term.an_element
2ab534bsome_elements for terms
3e71113_an_element_ and some_elements for asymptotic rings
43fd06ddelete stop-option from some_elements
4f294ceimprove product_diagonal so that input is read only when needed

comment:3 Changed 4 years ago by dkrenn

  • Description modified (diff)

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

  • Branch changed from u/dkrenn/asy/an_element to u/behackl/asy/an_element
  • Commit changed from 4f294ce9023fc4908bb3ad67228f900111826158 to 9c1fc0e5ae62f1fdfc59acea333ea9df9637e0b7
  • Reviewers set to Benjamin Hackl

I've reviewed your changes, merged the latest asy/asymptoticExpression into this branch and applied a tiny reviewer's patch (somehow, 'exact was pasted where it definitely should not). In principal, everything looks good to me and the doctests pass.

However, do you think that asymptotic_term.py really is the best place for the product_diagonal function? I do understand that it is required as a helper function there -- but nevertheless, from my point of view, the function is sufficiently general so that it could also live, for example, in src/sage/misc/misc.py.

What do you think?


Last 10 new commits:

43fd06ddelete stop-option from some_elements
4f294ceimprove product_diagonal so that input is read only when needed
186ecfcsimplified doctests: removed some unneccessary imports
aa4c647Merge branch 'u/dkrenn/asy/asymptoticExpression' into asy/asymptoticExpression
5911564typo fixed and line break introduced
7aa3e60`QQ` --> `\mathbb{Q}`
3e2a7b3typo fixed
f118772some SEEALSO-blocks added
9354773Merge branch 'asy/asymptoticExpression' into asy/an_element
9c1fc0ecleanup documentation: strange 'exact removed

comment:5 Changed 4 years ago by dkrenn

  • Branch changed from u/behackl/asy/an_element to u/dkrenn/asy/an_element

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

  • Commit changed from 9c1fc0e5ae62f1fdfc59acea333ea9df9637e0b7 to 3a44a01c5f3d39c51071d57f7ae5648ba63dbba8

Replying to behackl:

I've reviewed your changes, merged the latest asy/asymptoticExpression into this branch and applied a tiny reviewer's patch (somehow, 'exact was pasted where it definitely should not). In principal, everything looks good to me and the doctests pass.

Thanks.

However, do you think that asymptotic_term.py really is the best place for the product_diagonal function? I do understand that it is required as a helper function there -- but nevertheless, from my point of view, the function is sufficiently general so that it could also live, for example, in src/sage/misc/misc.py.

I would keep it here, as it is needed (at the moment) only here.


Last 10 new commits:

6d01815Merge branch 'asy/asymptoticTerm' into asy/asymptoticExpression
f6c4d80doctests fixed
c7afc80building documentation fixed
9305ca5add . after seealso-lists
50d7166\mathbb{Q} --> \QQ
2f8146ddoctests: add growth_group= and coefficient_ring= to AsymptoticRing(...) generation
6456300minor rewording in docstring
4e7b080Merge branch 'u/dkrenn/asy/asymptoticExpression' into asy/asymptoticExpression and fixed some merge conflicts
ae47bcbMerge remote-tracking branch 'origin/u/behackl/asy/asymptoticExpression' into t/19048/asy/an_element
3a44a01fix imports to make doctests pass

comment:7 Changed 4 years ago by dkrenn

  • Status changed from new to needs_review

comment:8 Changed 4 years ago by git

  • Commit changed from 3a44a01c5f3d39c51071d57f7ae5648ba63dbba8 to 1108cfc6af6cf8049e3d818270bc321c7f16ec4b

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

7b8c1a0fixed a conversion issue for terms with coefficient
83f74d3Merge branch 'asy/asymptoticTerm' into asy/asymptoticExpression
1c81c12language oddities fixed
032d8b8Merge branch 'asy/growthGroup' into asy/growthGroup-factory
3a05be7Merge tag '6.9.beta5' into t/17600/asy/growthGroup
58f931dadd asymptotic_expansions index
9d6f2daMerge branch 't/17600/asy/growthGroup' into t/18930/asy/growthGroup-factory
6da5adeMerge branch 't/18930/asy/growthGroup-factory' into t/17715/asy/asymptoticTerm
2c1c39dMerge branch 't/17715/asy/asymptoticTerm' into t/17716/asy/asymptoticExpression
1108cfcMerge branch 't/17716/asy/asymptoticExpression' into t/19048/asy/an_element

comment:9 Changed 4 years ago by dkrenn

Merged 6.9.beta5

comment:10 Changed 4 years ago by dkrenn

  • Component changed from symbolics to asymptotic expansions

comment:11 Changed 4 years ago by git

  • Commit changed from 1108cfc6af6cf8049e3d818270bc321c7f16ec4b to e9ed00562a257b8fff6ae85384944c40c48f13f1

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

e9ed005fix docstring (:: instead of :)

comment:12 Changed 4 years ago by behackl

  • Branch changed from u/dkrenn/asy/an_element to u/behackl/asy/an_element
  • Commit changed from e9ed00562a257b8fff6ae85384944c40c48f13f1 to 7bc65a7dbba9451510759e5575f70ac4a52e3587

Merged positively reviewed dependencies into this branch and fixed a simple conflict.


Last 10 new commits:

17e921fremove sorted_set_by_tuple
7af4b6bremove reverse keyword from shells
a49a20dadd comment in code to make it clear what happens
89b8209change left/right to self/other
1d52f6cadd a note to the set operations methods
3b3b2fbobject --> SageObject
572a95dMerge branch 'asy/mutable-poset' into asy/asymptoticExpression
5f54aecexplicitly forbid coercion from MutablePoset into the AsymptoticRing
4def011Merge branch 'asy/asymptoticExpression' into asy/an_element
7bc65a7base_ring --> base_ring()
Last edited 4 years ago by behackl (previous) (diff)

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

  • Reviewers changed from Benjamin Hackl to Benjamin Hackl, Clemens Heuberger

Perhaps the name product_diagonal could be changed to refer to the Cantor pairing function?

There is a module on multidimensional enumeration, but apparently, it only deals with finite iterators.

comment:14 in reply to: ↑ 13 Changed 4 years ago by dkrenn

Replying to cheuberg:

Perhaps the name product_diagonal could be changed to refer to the Cantor pairing function?

There is a module on multidimensional enumeration, but apparently, it only deals with finite iterators.

follow up #19319 (is now a dependency of this ticket).

Last edited 4 years ago by dkrenn (previous) (diff)

comment:15 Changed 4 years ago by dkrenn

  • Branch changed from u/behackl/asy/an_element to u/dkrenn/asy/an_element

comment:16 Changed 4 years ago by git

  • Commit changed from 7bc65a7dbba9451510759e5575f70ac4a52e3587 to ba99790d116341acfd9a691ac0a2f26f705aa988

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

ba99790use new product_cantor_pairing and delete old product_diagonal

comment:17 Changed 4 years ago by dkrenn

  • Dependencies changed from #17716, #19047 to #17716, #19047, #19319

comment:18 Changed 4 years ago by cheuberg

  • Status changed from needs_review to needs_work

Doctests pass on 6.9.beta5. On 6.9.rc0, they do not (cf. patchbot). After merging all dependencies into 6.9.rc0, there is still one failing doctest:

sage -t src/sage/rings/asymptotic/asymptotic_ring.py
**********************************************************************
File "src/sage/rings/asymptotic/asymptotic_ring.py", line 655, in sage.rings.asymptotic.asymptotic_ring.AsymptoticExpression.__pow__
Failed example:
    (y^2+O(y))^(-2)
Expected:
    Traceback (most recent call last):
    ...
    TypeError: unsupported operand parent(s) for '/':
    'Asymptotic Ring <y^ZZ> over Integer Ring' and 'Asymptotic
    Ring <y^ZZ> over Integer Ring'
Got:
    <BLANKLINE>
    Traceback (most recent call last):
    ...
    CoercionException: Infinite loop in action of Integer Ring (parent <type 'sage.rings.integer_ring.IntegerRing_class'>) and Asymptotic Ring <y^ZZ> over Integer Ring (parent <class 'sage.rings.asymptotic.asymptotic_ring.AsymptoticRing_with_category'>)!

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

Merging #19068 would solve the problem, but perhaps you could have a look whether there is some deeper problem before.

comment:20 Changed 4 years ago by git

  • Commit changed from ba99790d116341acfd9a691ac0a2f26f705aa988 to f0b1172fe8bb87a1a36ff61415cf7442ba319a30

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

abb08ffimprove _element_constructor_
78b9e96Trac #17716: additional doctest
055e35bTrac #17716: Fix ReSt error
e8e2501make entry in reference/index
e81d77dMerge branch 'u/dkrenn/asy/asymptoticExpression' of trac.sagemath.org:sage into t/19048/asy/an_element
9a77311deal with product(empty, infinite)
70185c3remove unneccesary try/except block
d4dec2bTrac #19319: alternative implementation
fde8e6dminor changes to code: spacings, PEP8, remove comment
f0b1172Merge branch 'u/dkrenn/product_cantor_pairing' of trac.sagemath.org:sage into t/19048/asy/an_element

comment:21 Changed 4 years ago by git

  • Commit changed from f0b1172fe8bb87a1a36ff61415cf7442ba319a30 to f6b06c9fdd58dbfa0f655dc0d3d24d0399abb46a

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

8894fceMerge branch 't/17716/asy/asymptoticExpression' into t/19068/asy/inversion
dc38b28Merge branch 'asy/asymptoticExpression' into asy/inversion
59e5d38Merge tag '6.9.rc0' into asy/inversion
f0b759aMerge branch 'asy/asymptoticExpression' into asy/inversion
b37740fremove code duplicate
bfc460afix removed keyword 'default_prec'
fc9bcf2add result to doctest
4edaa39slightly improve __pow__
c7023ddtodo-block with remark w.r.t. L-terms added
f6b06c9Merge branch 'u/behackl/asy/inversion' of trac.sagemath.org:sage into t/19048/asy/an_element

comment:22 Changed 4 years ago by dkrenn

  • Dependencies changed from #17716, #19047, #19319 to #17716, #19047, #19068, #19319
  • Status changed from needs_work to needs_review

comment:23 in reply to: ↑ 19 Changed 4 years ago by dkrenn

Replying to cheuberg:

Merging #19068 would solve the problem, but perhaps you could have a look whether there is some deeper problem before.

Merging #19068 is fine.

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

  • Status changed from needs_review to needs_work

#19319 now has completely new code which is guaranteed to produce merge conflicts here.

comment:25 Changed 4 years ago by cheuberg

  • Branch changed from u/dkrenn/asy/an_element to u/cheuberg/asy/an_element

comment:26 Changed 4 years ago by git

  • Commit changed from f6b06c9fdd58dbfa0f655dc0d3d24d0399abb46a to 617c5933907d132785ee1052473dfa808350ec04

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

c41435fRevert "copy code from #19048"
c655f9fTrac 19319: Cantor iteration of cartesian products
4a52a84Trac 19319: fix doctests
3c5af3bTrac #19319: fix typo
c20bfe5Trac #19319: a.next() -> next(a) (Python3 compliance)
1fee722Trac #19319: added a few blanks
96c0366Trac 19319: return tuples + repeat argument
ceb1db5Trac #19048: Merge #19319
3fd53d6Trac #19048: rename product_cantor_pairing to cantor_product (see #19319)
617c593Trac #19048: Fix doctests (order in cantor_product changed)

comment:27 in reply to: ↑ 24 Changed 4 years ago by cheuberg

  • Status changed from needs_work to needs_review

I reviewed this code before #19319 had been factored out and did not have any objections to the part which is still in this ticket.

Replying to cheuberg:

#19319 now has completely new code which is guaranteed to produce merge conflicts here.

I now reverted all changes by the old branch of #19319, merged the new branch of #19319 and fixed code and doctests here.

Please cross-review this revert+merge+fix and set the ticket to positive_review if you are satisfied.

comment:28 Changed 4 years ago by behackl

  • Status changed from needs_review to positive_review

Thanks for your review and the merge of the changes at #19319. I cross-checked everything; your changes look fine; documentation builds and doctests pass.

Together with your previous review, this is positive_review.

comment:29 Changed 4 years ago by vbraun

  • Branch changed from u/cheuberg/asy/an_element to 617c5933907d132785ee1052473dfa808350ec04
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.