-
-
Notifications
You must be signed in to change notification settings - Fork 491
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
AsymptoticTerm #17715
Comments
Commit: |
Branch: u/behackl/asy/asymptoticTerm |
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
|
Branch pushed to git repo; I updated commit sha1. New commits:
|
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
|
Branch pushed to git repo; I updated commit sha1. New commits:
|
Branch pushed to git repo; I updated commit sha1. New commits:
|
Changed branch from u/behackl/asy/asymptoticTerm to u/dkrenn/asy/asymptoticTerm |
Reviewer: Daniel Krenn |
Last 10 new commits:
|
Changed author from Benjamin Hackl to Benjamin Hackl, Daniel Krenn |
Changed branch from u/dkrenn/asy/asymptoticTerm to u/behackl/asy/asymptoticTerm |
Last 10 new commits:
|
Dependencies: #17600 |
Changed keywords from none to gsoc15 |
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
|
comment:36
Replying to @behackl:
My impression when reviewing this ticket was that absorption is explained many times. Perhaps these explanations and examples could have been collected in one place (either in the module documentation or in the relevant methods in the base class). Then the documentation of all overriden classes could have included a link to the general explanation and only explain the particularities of the respective class. Having the explanation in the docstring of a method of the base class would have the advantage that users of This module is of rather technical nature anyway and the end user will hopefully rarely look into it. Thus reducing the number of occurrences of IMHO, having
Could you add one sentence to the documentation? Something like "Setting
I think that if the current code expects the growth group to be a
The attached comment should also be changed. Having a look again, I now partially understand why
I do not know yet what the usecase for comparison of elements of equal growth was supposed to be. I am not sure, though, whether Are distinct conventions for
Could someone elaborate on that?
Why? Including this ticket, we have 74 classes with
and one of them (this ticket) has it as a property:
I'd like to see more convincing arguments before breaking consistency with 73 other sage modules.
done. I have one further question not discussed so far:
|
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:38
Replying to @cheuberg:
I think that you are right. I've collected the documentation for
Done.
It does so now.
I don't think that conversion is at work in this particular section---in any case, I've rewritten these doctests slightly in order to better reflect what we want to demonstrate.
In fact, we only need the comparison of the underlying growth. I've deleted As this is--as you already said--a rather technical class, I think that it can be justified if
Mea culpa, I was wrong. Later on, we renamed
Well, we currently only have three cases when
Benjamin |
Changed branch from u/behackl/asy/asymptoticTerm to u/cheuberg/asy/asymptoticTerm |
comment:40
Replying to @behackl:
First, I do not know what the next tickets want to see, whether Second, where is the implementation of equality of growth terms? If in doubt, I would say that Apart from that, I reviewed your changes, added a few reviewer commits (please cross-review). New commits:
|
comment:41
Replying to @cheuberg:
I agree, once again. We should preserve that
I've added this, as well as a short note in the module description how terms are compared.
Ah! These did not exist on this ticket -- I have added them as well, as they logically belong here.
I reviewed your changes, and added the points from above (please cross-review again Benjamin Last 10 new commits:
|
Changed branch from u/cheuberg/asy/asymptoticTerm to u/behackl/asy/asymptoticTerm |
Changed branch from u/behackl/asy/asymptoticTerm to u/cheuberg/asy/asymptoticTerm |
Changed branch from u/cheuberg/asy/asymptoticTerm to |
Asymptotic terms are expressions like O(n2), 7 * n * 2n, or 42 * n * log(n). They build upon the asymptotic growth elements from #17600, which are elements like n2, n*2n and n * log(n) (that is, they handle only the asymptotic growth).
All asymptotic terms have an attribute 'growth' (which is some growth element), and then they may have additional information (like, for example, a coefficient in the case of exact terms).
Currently, we implemented the following asymptotic terms (plus their monoid parents):
GenericTerm::
Implements the base structure of asymptotic terms.
OTerm::
Class for big O terms. These terms may "absorb" other asymptotic terms with weaker or equal growth.
TermWithCoefficient::
Generic base class for asymptotic terms with coefficient. Base class for asymptotic exact terms and asymptotic L terms.
ExactTerm::
Class for asymptotic exact terms. These terms correspond to symbolic expressions like, for example, 2 * x3, 7 * x-2/5, or 42 * x1/3 * log(x).
Asymptotic terms may be multiplied and absorbed; addition will be handled by AsymptoticExpression.
See meta-ticket #17601 for the planned structure.
Depends on #17600
Depends on #18930
Depends on #19237
CC: @dkrenn @cheuberg @nathanncohen @videlec @sagetrac-tmonteil
Component: asymptotic expansions
Keywords: gsoc15
Author: Benjamin Hackl
Branch/Commit:
aea0ae8
Reviewer: Daniel Krenn, Clemens Heuberger
Issue created by migration from https://trac.sagemath.org/ticket/17715
The text was updated successfully, but these errors were encountered: