#15149: Bug in pickling of toric varieties, II
-------------------------------------+-------------------------------------
Reporter: jkeitel | Owner:
Type: defect | Status: new
Priority: major | Milestone: sage-6.0
Component: algebraic | Resolution:
geometry | Merged in:
Keywords: toric | Reviewers:
Authors: | Work issues:
Report Upstream: N/A | Commit:
Branch: | 15a41647c634421769963f6b6ccabe65c7907789
u/jkeitel/toric_pickling_2 | Stopgaps:
Dependencies: #15050 |
-------------------------------------+-------------------------------------
Comment (by nbruin):
Replying to [comment:4 vbraun]:
> In theory, I would agree that one needs to implement `__reduce__`
(basically, for all classes that inherit from !UniqueRepresentation).
In fact, `UniqueRepresentation` should be particularly easy to pickle: the
weak cache that gets used to see if objects already exist has the
parameters used to instantiate them already cached as a key! If the
objects in there have sane pickling themselves (and they should, because
they're hashable, and hence fairly immutable, so their important
properties haven't changed since they have been created, so any
potentially circular stuff can go into setstate), a reasonable set of
pickling parameters is already there! It may be worthwhile to see if
`UniqueRepresentation` could offer a default `reduce` that gets a little
towards that concept. I think it might make sense, at least for classes
that are also `EqualityById`.
> 1. It is really hard to doctest. Not only must you restore all the data
for the hash, but if there is a hash collision then you also need all the
data for `__cmp__()`.
If our design decisions are being affected by whether something is hard to
doctest then we really have cart and horse in the wrong order.
Let's see. `ToricVariety_field` has these creation parameters:
{{{
fan, coordinate_names, coordinate_indices, base_field
}}}
You got to be able to decide equality based on these.
The last 3 should definitely not introduce circularities in their
construction phases. Let's look at `fan`:
{{{
cones, rays, lattice
}}}
Again, those should be the only parameters involved in the construction
phase of a fan. The rest can go into setstate.
I don't know exactly what things you need to define `cones, rays,
lattice`, but it seems like those should also be strictly non-circularly
constructible (any caches you might want to pickle can go into
`setstate`).
> 2. In practice, Python doesn't support circular `__reduce__` so we'll
end up just triggering that bug.
And every time you're getting that (and we'd hope we run into an error
report, because the silent failures are really painful), it's an
indication that you got your `construction/setstate` balance wrong.
Given that the very nature of Python (and any normal programming
language), any circular reference is always inserted by ''modifying'' and
existing object(*). That's what `setstate` in pickling is for.
(*) There's of course Simon's example
{{{
class circ(object):
def __init__(self,i)"
self.i=i
self.d={self:1}
}}}
which indeed seems like circularity-upon-construction, but the key here is
that no circular input data is needed to complete the construction.
--
Ticket URL: <http://trac.sagemath.org/ticket/15149#comment:5>
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/groups/opt_out.
