is_exact for asymptotic ring

[behackl]

This ticket shall implement a method that returns wheter all terms of some asymptotic expansion are exact terms.

CC: @cheuberg @dkrenn

Component: asymptotic expansions

Author: Benjamin Hackl

Branch/Commit: da853f8

Reviewer: Daniel Krenn, Clemens Heuberger

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

[behackl]

Commit: 550e717

[behackl]

Branch: u/behackl/asy/misc-improvements

[behackl]

New commits:

cc0137d	implement is_exact
550e717	implement _latex_

[behackl]

comment:2

Note: this is on top of 7.1.beta1.

Any more suggestions for practical improvements?

[cheuberg]

comment:3

Replying to @behackl:

Note: this is on top of 7.1.beta1.

Any more suggestions for practical improvements?

no; they could go into a separate ticket anyway.

[dkrenn]

comment:4

Replying to @cheuberg:

they could go into a separate ticket anyway.

+1 for separate tickets.

[dkrenn]

comment:5

I had a quick look at the code:

cc0137d	implement is_exact

IMHO inefficient; has_same_summands should do better, since no new mutable posets have to be created.

550e717	implement _latex_

SR uses the commutativity of the multiplication sometimes, so LaTeX-output might differ from repr-output. I suggest to implement _latex_ for terms and growth groups as they know what to do.

[behackl]

comment:6

Replying to @dkrenn:

I had a quick look at the code:

cc0137d	implement is_exact

IMHO inefficient; has_same_summands should do better, since no new mutable posets have to be created.

I'd even use _has_same_summands_, the parent is certainly the same so this is even more efficient. Will change later.

550e717	implement _latex_

SR uses the commutativity of the multiplication sometimes, so LaTeX-output might differ from repr-output. I suggest to implement _latex_ for terms and growth groups as they know what to do.

That was/is my original plan, and this is also the reason why I didn't set this to needs_review. ;-) However, I wanted to have nice (growth-ordered) latex-output quickly in order to check the output of the singularity analysis generator; and this was the simplest implementation I could think of. The code is just dumped here.

I'll also split this ticket into two.

[behackl]

Changed branch from u/behackl/asy/misc-improvements to u/behackl/asy/is_exact

[behackl]

New commits:

09a3db2	implement is_exact
26639e2	improve performance of is_exact

[behackl]

Changed commit from 550e717 to 26639e2

[dkrenn]

comment:8

Typo: "Nothin.", but you could remove the INPUT-block completely. Otherwise looks good. Still have to wait until I have a working 7.1.beta1.

PS: Author missing

[dkrenn]

Reviewer: Daniel Krenn

[cheuberg]

Changed reviewer from Daniel Krenn to Daniel Krenn, Clemens Heuberger

[cheuberg]

comment:9

Apart from that: while the routine is hardly efficiency critical, it is somewhat inefficient to construct exact_part and then comparing with the original just to check whether all summands are ExactTerms. If this is to ensure that future extensions of the term monoids won't introduce inconsistencies, then there could be a helper method is_exact.

[dkrenn]

comment:10

Replying to @cheuberg:

Apart from that: while the routine is hardly efficiency critical, it is somewhat inefficient to construct exact_part and then comparing with the original just to check whether all summands are ExactTerms. If this is to ensure that future extensions of the term monoids won't introduce inconsistencies, then there could be a helper method is_exact.

True. Maybe something like all(T.is_exact() for T in self.summands)...

[cheuberg]

comment:12

Replying to @dkrenn:

True. Maybe something like all(T.is_exact() for T in self.summands)...

That's what I meant.

[behackl]

comment:13

I'm not a very big fan of having a method is_exact for terms, where this would just return isinstance(self, ExactTerm). I think that this would just add an unneccessary layer of function calls.

Are you against something like all(isinstance(T, ExactTerm) for T in self.summands)?

[sagetrac-git]

Changed commit from 26639e2 to da853f8

[sagetrac-git]

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

9042dab	implement is_exact for terms
f5bdb8f	replace isinstance(..., ExactTerm) with is_exact
f65bc42	remove INPUT in docstring
da853f8	implement expansion.is_exact() by using term.is_exact()

[behackl]

Author: Benjamin Hackl

[behackl]

comment:15

I've changed my mind: to keep our style of programming consistent, I have implemented an is_exact-method for terms and replaced all isinstance(term, ExactTerm)-statements by term.is_exact().

Back to needs_review.

[dkrenn]

comment:16

Seems to be fine now.

[vbraun]

Changed branch from u/behackl/asy/is_exact to da853f8