#17693: mutable poset: a data structure for asymptotic expressions
-------------------------------------+-------------------------------------
Reporter: dkrenn | Owner:
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-6.9
Component: asymptotic | Resolution:
expansions | Merged in:
Keywords: asymptotics | Reviewers: Benjamin Hackl,
Authors: Daniel Krenn | Clemens Heuberger
Report Upstream: N/A | Work issues:
Branch: u/dkrenn/asy | Commit:
/mutable-poset | 4e73b4576e1e949b7257d858fbeb958152df9f66
Dependencies: | Stopgaps:
-------------------------------------+-------------------------------------
Changes (by dkrenn):
* commit: 4deddbd1a09e0e60273797cb89becdc651325b9c =>
4e73b4576e1e949b7257d858fbeb958152df9f66
Comment:
Replying to [comment:43 cheuberg]:
> Thank you for your changes. I added three commits. See my remaining
comments below.
Cross-review...ok.
> > > 7. `MutablePosetShell.key`: I am surprised that the key is not
cached. I imagine that many comparisons will be needed in the lifetime of
a `MutablePoset`, and in every case, the property key has to be resolved,
which calls the property poset, which calls `get_key` of the poset.
> >
> > Now it is cached (see also follow up ticket #19281).
>
> I rather thought about calling `key` in `__init__` as I guess that the
key will be needed at least once in the lifetime of every
`MutablePosetShell`.
Oh...yes, I agree; changed.
> > > 18. `MutablePosetShell._copy_all_linked_`: I do not understand why
you test `oo == P.oo`: I think that `oo` is an element of the new poset
`Q`.
> >
> > `oo == P.oo` tests that `Q.oo` is mapped to ``P.oo``.
>
> wouldn't `oo.is_oo` do it without comparing shells of different posets,
which might be confusing?
True. Changed.
> > > So `oo is Q.oo` would be more interesting?
> >
> > `oo is Q.oo` is `False` since `oo` is not in `Q`, but just a copy of
the `oo` in `P` with parent-poset `Q`.
>
> that would be good to test (and comment on).
Inserted.
> > Inserting this into `Q` is done in `_copy_shells_`.
> >
> > > The current test demonstrates that the poset is not used in
comparison, so that would rather belong to `.eq`?
> >
> > I do not understand what you mean (but it is already late...).
>
> I mean that shells `e` and `f` might be equal even if they do belong to
different posets; this might be an interesting doctest or example for
equality.
Now I understand what you mean. Inserted a doctest there.
> > > 22. `MutablePosetShell._search_covers_`: According to Wikipedia, a
cover is what is called an upper cover here. This is in contrast to the
default behaviour here.
> >
> > `_search_covers_` does not exist anymore (see 27).
>
> This does not solve the problem.
>
> What about renaming the method `covers` to `lower_covers` (and adapting
the docstring slightly, removing "upper " from the one-sentence-
description as well as from the definition?) For symmetry, a method
`upper_covers` would be nice.
Good idea; changed and inserted `upper_covers`.
> > > 55. `MutablePoset.intersection` and
`MutablePoset.intersection_update`: see comments on `union` and
`union_update`
> >
> > Done.
>
> Remove comment on non-commutativity? merge does not play a role here,
and keys might be covered in the previous sentence?
Removed.
> 63. `MutablePoset.update_union`: "Due to keys and a merge function...
this operation might not be commutative": This method is non-commutative
by definition, as `self` is changed and `other` is not. So perhaps:
"`left.update_union(right)`" and "`right.update_union(left)` might result
in different posets"? The same for other `update_...` methods where non-
commutativity might be surprising.
Changed.
----
New commits:
||[http://git.sagemath.org/sage.git/commit/?id=f7bd83a0f256e186fe5f8ecc9c4501d0643f7811
f7bd83a]||{{{Trac #17693, comment 43, 7: do caching of key in
__init__}}}||
||[http://git.sagemath.org/sage.git/commit/?id=a554d697526397fca280239fa11b210cbec94c80
a554d69]||{{{rac #17693, comment 43, 18: use .is_oo to test for oo in
doctest}}}||
||[http://git.sagemath.org/sage.git/commit/?id=3ac7ed1b8f318620cbeee52f570d6e8ef7bb23f9
3ac7ed1]||{{{rac #17693, comment 43, 18: add a doctest "oo is Q.oo"}}}||
||[http://git.sagemath.org/sage.git/commit/?id=2f675b3a6fff5e50770e85aedc23f38afbe275f0
2f675b3]||{{{Trac #17693, comment 43, 18: add a doctest in eq (comparing
elements in different posets)}}}||
||[http://git.sagemath.org/sage.git/commit/?id=f064a3b6434551b97a0bbb36d918b8b2d1ffe3d6
f064a3b]||{{{Trac #17693, comment 43, 22: covers --> lower_covers,
upper_covers}}}||
||[http://git.sagemath.org/sage.git/commit/?id=bea925cec11160275c4b3357c25b0f3d157d482e
bea925c]||{{{Trac #17693, comment 43, 55: remove comment on non-
commutativity}}}||
||[http://git.sagemath.org/sage.git/commit/?id=4e73b4576e1e949b7257d858fbeb958152df9f66
4e73b45]||{{{Trac #17693, comment 43, 63: extend/rewrite non-commutativity
note}}}||
--
Ticket URL: <http://trac.sagemath.org/ticket/17693#comment:45>
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.
