#19083: AsymptoticRing: cleanup, some improvements, documentation
-------------------------------------+-------------------------------------
Reporter: dkrenn | Owner:
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-6.10
Component: asymptotic | Resolution:
expansions | Merged in:
Keywords: | Reviewers: Clemens Heuberger
Authors: Daniel Krenn | Work issues:
Report Upstream: N/A | Commit:
Branch: | ccf7dcad7f22efb681be2fb49bc8921a30e1de34
u/cheuberg/asy/prototype | Stopgaps:
Dependencies: #19094 |
-------------------------------------+-------------------------------------
Changes (by cheuberg):
* reviewer: => Clemens Heuberger
* milestone: sage-6.9 => sage-6.10
Comment:
Due to many interdependencies, I review this ticket and its dependency
#19094 in one go. I pushed some reviewer commits here and add my comments
here. The aim is to eventually set both tickets to `positive_review` at
the same time.
Up to now, I reviewed the following files:
||`src/doc/en/reference/rings/asymptotic_expansions_index.rst` || 50
`+++-` ||
||`src/sage/data_structures/mutable_poset.py` || 3
`+-` ||
||`src/sage/rings/all.py` || 2
`+-` ||
||`src/sage/rings/asymptotic/all.py` || 3
`+-` ||
||`src/sage/rings/asymptotic/growth_group.py` || 1779
`+++++++++++++++++++++++++++++++++++`
`++++++++++++++++++++++++++++++++++++` `+++++++++++++++++++++`
`-----------------------------------------` ||
||`src/sage/rings/asymptotic/misc.py` || 476
`++++++++++++++++++++++++++++++++++++` ||
Here are my comments on these files:
1. `misc.split_str_by_op`:
- parameter `op`: is special treatment of `^^` worthwhile when `op
='^'` or is that a special case for `*`?
- parameter `strip_parentheses` not tested
- document and test `op is None`. This is used in
`growth_group.Variable.__init__`
2. `misc.repr_op`: what about `-` and `/`? Why don't they also induce
parentheses?
{{{
sage: from sage.rings.asymptotic.misc import repr_op
sage: repr_op('a-b', '^', 'c')
'a-b^c'
sage: repr_op('a+b', '^', 'c')
'(a+b)^c'
}}}
3. `growth_group.is_lt_one`: this is a module level function, but doctests
use it as a method.
- mark doctest as indirect
- document this fact.
4. `growth_group.log`:
- see comments on `growth_group.is_lt_one`.
- error "results in a sum" is not tested.
5. `growth_group.log_factor`:
- why `from growth_group import GenericGrowthGroup`?
- see comments on `growth_group.is_lt_one`
6. `GenericGrowthElement.__ne__`: is the "Note" box actually needed?
7. `GenericGrowthElement._rpow_element_`:
- when is the output `None`?
- I do not understand "it lives (in contrast to rpow) in its own
group".
8. `GenericGrowthGroup._create_element_via_parent_`: I do not understand
the explanation. Is "via" a good part of the name of the method?
9. `GenericGrowthGroup.gens`:
- why "monomial" growth group in the generic group?
- why `MonomialGrowthElement`?
10. `MonomialGrowthElement.__pow__`: Is the following intentional:
{{{
sage: P = GrowthGroup('x^ZZ')
sage: x = P.gen()
sage: a = x^7; a
x^7
sage: a^(1/7)
x
sage: (a^(1/7)).parent()
Growth Group x^QQ
}}}
I understand that `((1/7)*7).parent()` is `QQ`, but in that case, the
user explicitly left the integers. It is not clear to me whether this is
also the case here.
11. `MonomialGrowthElement._log_factor_`:
- The parameter `base` is ignored in the second `if`:
{{{
sage: G = GrowthGroup('exp(x)^ZZ*x^ZZ')
sage: e1 = G('exp(x)'); e1
exp(x)
sage: log(e1, base=2)
x
}}}
- In some cases, computation fails although it would be feasible:
{{{
sage: G = GrowthGroup('(e^x)^ZZ*x^ZZ')
sage: e1 = G('e^x'); e1
e^x
sage: var('e')
sage: log(e1^2, base=e^2)
Traceback (most recent call last):
...
ArithmeticError: Cannot build log((e^x)^2) since log(e^x) is not in Growth
Group (e^x)^ZZ * x^ZZ.
}}}
12. `MonomialGrowthElement._rpow_element_`:
- I do not understand "it lives (in contrast to rpow()) in its own
group."
- Parameter `base` is not tested (with an actual result).
- The documentation does not explain why `2^x` is illegal here (it
seems that this method operates only if the result can be a monomial
growth group element, not necessarily in the same growth group as `self`).
13. `ExponentialGrowthElement._repr_`: what is the motivation behind
checking for `-/^`? This leads to the following (perhaps stupid) example:
{{{
sage: from sage.rings.asymptotic.growth_group import
ExponentialGrowthGroup
sage: G = ExponentialGrowthGroup(ZZ['x'], 'y'); G
Growth Group ZZ[x]^y
sage: G('(1-x)^y')
(-x + 1)^y
sage: G('(1+x)^y')
x + 1^y
}}}
14. `GrowthGroupFactory`: the documentation sounds like
`ignore_variables=('e',)` would only be set when no keywords are given.
However, this applies even if only `ignore_variables` is not set.
--
Ticket URL: <http://trac.sagemath.org/ticket/19083#comment:60>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
