#18587: cartesian products of growth groups
-------------------------------------+-------------------------------------
Reporter: dkrenn | Owner:
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-6.9
Component: asymptotic | Resolution:
expansions | Merged in:
Keywords: asymptotics, | Reviewers: Clemens Heuberger
gsoc15 | Work issues:
Authors: Benjamin Hackl, | Commit:
Daniel Krenn | f88d3ead2feadd76bf3fccbd6488f7d0c36c501d
Report Upstream: N/A | Stopgaps:
Branch: u/behackl/asy |
/growth-group-cartesian |
Dependencies: #18223, #18586, |
#18930 |
-------------------------------------+-------------------------------------
Comment (by behackl):
Replying to [comment:18 cheuberg]:
> I reviewed this ticket, but without reviewing the dependency on #18223,
so consider this as a conditional review.
>
> I added a few reviewer commits. There is still a merge conflict with
6.9.rc0 caused by #17715.
>
Hello Clemens! Thank you for your review!
The merge conflict with `6.9.rc0` has been resolved.
> Here are my other comments.
>
> 1. Introduction
> - I suggest to add a section heading before the note box (e.g.
"Creation of Growth Groups") and to downgrade the note box to be an
introductory paragraph of this section.
> - Last sentence: "It is mathematically nonsense": explain how the
order of factors of a single variable matters.
Done and done.
> 2. `Variable.__init__`: `ValueError` "is not a valid name": not tested
Removed this particular piece of code; the check is not necessary: the
parser only allows strings that are valid identifiers (see my answer to
3.).
> 3. `Variable.extract_variable_names`:
> - I am surprised that a more or less complete symbolic expression
parser is needed here. Would it not be simpler to use `SR(s)` and then
call `.variables()`, possibly on subexpressions in order to detect
duplicates? It seems that `SR(s)` would create the symbolic variables on
the fly, but does not inject them into the global name space, so this
seems to be what is needed here.
Changed this parser to the symbolic ring.
> - I do not think the following is intentional:
> {{{
> sage: Variable.extract_variable_names('a + (b + c) + d')
> ('b', 'c', 'd')
> }}}
Indeed, this was not intentional. With the symbolic ring as the variable
parser, this does not happen. I've added this as a doctest.
> - Why does the regular expression for number contain a `$` but no
`^`?
This is now irrelevant.
> 4. `GenericGrowthGroup.gens_monomial`: Replace "is only implemented for"
by "must be implemented in"
Done.
> 5. `MonomialGrowthGroup`: Is the following intentionally allowed:
> {{{
> sage: from sage.rings.asymptotic.growth_group import MonomialGrowthGroup
> sage: MonomialGrowthGroup(ZZ, 'x+y')
> Growth Group x+y^ZZ
> }}}
Yes, this is intentional. On this ticket, the representation is ugly, but
this is fixed in #19083.
> 6. `MonomialGrowthGroup._convert_`:
> - Is the following intentional:
> {{{
> sage: import sage.rings.asymptotic.growth_group as agg
> sage: P = agg.MonomialGrowthGroup(ZZ, 'x')
> sage: P(1)
> 1
> sage: P('x^2')
> x^2
> sage: P('1')
> Traceback (most recent call last):
> ...
> ValueError: Cannot convert 1.
> sage: G('x^)42(')
> x^42
> }}}
The first two statements work as intended. The others are fixed here.
> - As in 3a, couldn't the string parsing be done by the symbolic ring?
Yes. Done.
> 7. `GrowthGroupFactory.create_key_and_extra_argument`: Again, my
suggestion is to delegate the string parsing to the symbolic ring. Note
that `SR('x^ZZ')` happily accepts `ZZ` as a variable, so I imagine that
postprocessing here would not involve big problems. I do not read this
string parser here before the decision on its eventual fate.
Unfortunately, I think that this should not be parsed by the symbolic
ring: the cartesian product which we model with `*` is not commutative: if
the same variable is used, the growth groups are ordered
lexicographically. By using the symbolic ring, the input order might be
destroyed/ignored.
> 8. Add one or more links from `growth_group.py` to
`growth_group_cartesian.py`.
We took extensive care with respect to the documentation on #19083. There,
some links have been added.
> 9. `CartesianProductFactory`:
> - indicate (somewhere, probably at a more prominent place) what order
is chosen automatically (apparently, lexicographic if the variables are
equal, component-wise otherwise). That would also explain why order does
matter (cf. 1b).
I've added a note-block to the docstring of the class.
> - the example with "do not have disjoint variables" is somewhat
puzzeling, because the cartesian product of `A` and `B` was allowed.
I understand the confusion. However, I think that the error message is
quite helpful in this case:
{{{
sage: cartesian_product([A, E]); G
Traceback (most recent call last):
...
ValueError: Growth groups (Growth Group x^ZZ, Growth Group x^ZZ * y^ZZ) do
not have pairwise disjoint variables.
}}}
Mainly, this is because the cartesian product cannot decide which order to
use in this case. Also, I think that we should restrict building cartesian
products of growth groups such that a factor may only occur once
(currently, `x^ZZ * x^ZZ` is possible). I'll discuss this with Daniel.
> - (continuation of b while reading the code): apparently, sets of
variables of the factors must be equal or disjoint.
I've added this remark to the note-block in the docstring of the factory.
> - mark doctests as indirect
Done.
> 10. `GenericProduct`: mark doctests as indirect.
Done.
> 11. Check links `~sage.sets.cartesian_product.CartesianProductPosets`
and `sage.sets.cartesian_product.CartesianProductPosets.Element` (do not
work for me, possibly time intensive recreation of documentation would
help).
I corrected the links (we shifted some code in #18223), they work for me
now -- however, that should not have affected you. (Sometimes, the
documentation really wants to be built from scratch. `:-)`)
> 12. `GenericProduct._coerce_map_from_`: don't the factors coerce into
the product?
> {{{
> sage: from sage.rings.asymptotic.growth_group import GrowthGroup
> sage: G = GrowthGroup('x^ZZ')
> sage: H = GrowthGroup('log(x)^ZZ')
> sage: K = cartesian_product([G, H])
> sage: K.has_coerce_map_from(G)
> False
> sage: K.has_coerce_map_from(H)
> False
> }}}
No, not in general. However, due to our special structure (and as soon as
we actually disallow copies of the same growth group in a cartesian
product) this can be implemented. This should be handled in our coercion
ticket, #19073.
> 13. Expand documentation of `UnivariateProduct` and
`MultivariateProduct`: at least, include a link to `GenericProduct` and
explain the choice of order.
Done.
--
Ticket URL: <http://trac.sagemath.org/ticket/18587#comment:21>
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.
