Opened 4 years ago
Closed 4 years ago
#19981 closed enhancement (fixed)
is_exact for asymptotic ring
Reported by: | behackl | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-7.1 |
Component: | asymptotic expansions | Keywords: | |
Cc: | cheuberg, dkrenn | Merged in: | |
Authors: | Benjamin Hackl | Reviewers: | Daniel Krenn, Clemens Heuberger |
Report Upstream: | N/A | Work issues: | |
Branch: | da853f8 (Commits) | Commit: | da853f87105d7e8ec4883141e1a98911cb91900c |
Dependencies: | Stopgaps: |
Description (last modified by )
This ticket shall implement a method that returns wheter all terms of some asymptotic expansion are exact terms.
Change History (17)
comment:1 follow-up: ↓ 5 Changed 4 years ago by
- Branch set to u/behackl/asy/misc-improvements
- Commit set to 550e7178b91b312f1513220f05045efb62dd5eb4
comment:2 follow-up: ↓ 3 Changed 4 years ago by
Note: this is on top of 7.1.beta1
.
Any more suggestions for practical improvements?
comment:3 in reply to: ↑ 2 ; follow-up: ↓ 4 Changed 4 years ago by
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.
comment:4 in reply to: ↑ 3 Changed 4 years ago by
comment:5 in reply to: ↑ 1 ; follow-up: ↓ 6 Changed 4 years ago by
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.
comment:6 in reply to: ↑ 5 Changed 4 years ago by
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.
comment:7 Changed 4 years ago by
- Branch changed from u/behackl/asy/misc-improvements to u/behackl/asy/is_exact
- Commit changed from 550e7178b91b312f1513220f05045efb62dd5eb4 to 26639e22157016fa1410ac8e14affd391779b6b9
- Description modified (diff)
- Status changed from new to needs_review
- Summary changed from asymptotic expansions: minor improvements to is_exact for asymptotic ring
comment:8 Changed 4 years ago by
- Reviewers set to Daniel Krenn
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
comment:9 follow-up: ↓ 10 Changed 4 years ago by
- Reviewers changed from Daniel Krenn to Daniel Krenn, Clemens Heuberger
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 ExactTerm
s. 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
.
comment:10 in reply to: ↑ 9 ; follow-up: ↓ 12 Changed 4 years ago by
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 areExactTerm
s. If this is to ensure that future extensions of the term monoids won't introduce inconsistencies, then there could be a helper methodis_exact
.
True. Maybe something like all(T.is_exact() for T in self.summands)
...
comment:11 Changed 4 years ago by
- Status changed from needs_review to needs_work
comment:12 in reply to: ↑ 10 Changed 4 years ago by
Replying to dkrenn:
True. Maybe something like
all(T.is_exact() for T in self.summands)
...
That's what I meant.
comment:13 Changed 4 years ago by
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)
?
comment:14 Changed 4 years ago by
- Commit changed from 26639e22157016fa1410ac8e14affd391779b6b9 to da853f87105d7e8ec4883141e1a98911cb91900c
comment:15 Changed 4 years ago by
- Status changed from needs_work to needs_review
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
.
comment:16 Changed 4 years ago by
- Status changed from needs_review to positive_review
Seems to be fine now.
comment:17 Changed 4 years ago by
- Branch changed from u/behackl/asy/is_exact to da853f87105d7e8ec4883141e1a98911cb91900c
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
implement is_exact
implement _latex_