#18290: enhanced sets and cartesian products
-------------------------+-------------------------------------------------
Reporter: | Owner:
vdelecroix | Status: needs_review
Type: | Milestone: sage-6.7
enhancement | Resolution:
Priority: major | Merged in:
Component: | Reviewers:
categories | Work issues:
Keywords: | Commit:
cartesian_product | 3cbe605b2bfc2d91f878fbde04bebc78d14f4cb7
Authors: | Stopgaps:
Vincent Delecroix |
Report Upstream: N/A |
Branch: |
public/18290 |
Dependencies: |
-------------------------+-------------------------------------------------
Comment (by vdelecroix):
Replying to [comment:44 nthiery]:
> I have been through Vincent's last commit. I pushed too minor fixes. I
have only a couple points I am wondering about:
>
> - Do we want to import ZZ in sage.sets.cartesian_product? Can we do a
local import or lazy import? Or is it just ok?
I think this is fine. I would find it strange to import it in
`sage.categories.*` but `sage.sets.cartesian_product` is lazily imported.
> - In `CartesianProduct._cartesian_product_of_elements`, it would be
semantically equivalent to use `zip` rather than `iterator.izip` since we
anyway build a tuple right after. Just curious, is one faster than the
other in practice (it's a tradeoff between building a list for nothing and
using another layer of iterators)?
Here you are not building the list from the pairs `(c,x)` but by calling
`c(x)`. So I guess that `izip` is always better. From the timing it is
very similar and `izip` gets much better as the size of the lists
increase. (moreover in `Python 3` the `izip` is just deleted and `zip`
becomes `izip`).
> - Please update the documentation of
`Sets.CartesianProducts.ParentMethods._cartesian_product_of_elements` to
specify that it should accept any iterable.
done.
> - _sets_keys(): do we really want an `IntegerRange` rather than a
`range`? I would imagine `IntegerRange` could be faster for containment
testing and slower for iteration, so again a tradeoff.
It is '''much''' faster for containment test and it is currently the only
way it is used in `sage.sets.cartesian_product` (the method
`cartesian_projection`). If the iteration is slower, then we should just
make it faster in `IntegerRange`!
> In any cases, I'll be busy all day and don't have a strong opinion on
either of the above points. So please make the choice you feel right. And
then I consider this as good to go.
I am running the long tests and will set positive review if everything is
fine.
Vincent
--
Ticket URL: <http://trac.sagemath.org/ticket/18290#comment:46>
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.
