#13605: Partition options and cleanup partitions documentation
-----------------------------------------------+----------------------------
Reporter: tscrim | Owner: sage-combinat
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-5.6
Component: combinatorics | Resolution:
Keywords: partition, options, output | Work issues:
Report Upstream: N/A | Reviewers: Andew Mathas
Authors: Travis Scrimshaw | Merged in:
Dependencies: #13074 #13762 #13840 #10193 | Stopgaps:
-----------------------------------------------+----------------------------
Comment (by tscrim):
Hey Andrew,
Sorry for the delay. I'm in Burma right now and while I can generally get
stable internet, some things work better than others.
Replying to [comment:12 andrew.mathas]:
> Could you please tell what the rationale is for having partition_options
and tableau_options defined in their own files. In the long term, we need
to be careful about "polluting" the file space so I don't think it is a
good idea to have these little code snippets in their own files.
>
> Similarly, I'm not convinced that it is a good idea to give the
partition options their own page/index entry in the manual (btw,
tableau_options doesn't seem to have a manual entry yet). I think that it
would be better to have the partition options discussed in a new section
at the top of partitions.rst.
My main thought was because it is not something strictly related to
partitions, but partition like objects (partition tuples and rigged
configurations, there might be others). Also because `partition.py` is so
long as is. Perhaps we could combine partition and tableau options in one
file `partition_tableau_options.py` and one manual page. I'm in favor of
having a separate manual page since it is about the display rather than
the functionality, although it probably should be flushed out more with
more examples and discussion.
Replying to [comment:13 andrew.mathas]:
> Hi Travis,
>
> Here are some more questions/issues. Consider
> {{{
> sage: a=Partitions(4)
> sage: b=Partitions(4,order='containment')
> sage: a
> Partitions of the integer 4
> sage: b
> Partitions of the integer 4
> sage: a == b
> False
> }}}
> This is correct, of course, but it is potentially confusing because a
and b look exactly the same when printed, I think that it would be better
if the ordering ordering on the class was returned by ``_repr_``, at least
when the default ordering is not being used. That is, I am suggesting the
following behaviour:
> {{{
> sage: a=Partitions(4)
> sage: b=Partitions(4,order='containment')
> sage: a
> Partitions of the integer 4
> sage: b
> Partitions of the integer 4 with the containment ordering
> sage: a == b
> False
> }}}
Agreed. I think this is done in a few other places too.
>
> Continuing this example:
> {{{
> sage: a[:]
> [[4], [3, 1], [2, 2], [2, 1, 1], [1, 1, 1, 1]]
> sage: a._list.sort(); a[:]
> [[1, 1, 1, 1], [2, 1, 1], [2, 2], [3, 1], [4]]
> }}}
> This, to me, just looks wrong: the default ordering should correspond to
the order in which the partitions are generated by the iterator. That is,
the default really should be 'rev_lex' or the iterator should change.
This smells like a bug since I would think `sort()` would used the
ordering on the objects (i.e. the list may not be actual partitions but
lists). However there is an small issue with the validity of the test
because not all orderings on partitions are total (linear) for a fixed
`n`. Maybe because of this, the entire idea of overriding the default
ordering is a white whale.
(Although this might also be why their doesn't seem to be a good ordering
in the symmetric functions (if that is, oh-joy-of-joys, I have to change a
lot of doctests again).)
> A second related issue is that in calling Partitions(4)[:], via a[:], we
have created the list of *all* partitions of 4 BUT b does not know about
this:
> {{{
> sage: a._list
> [[1, 1, 1, 1], [2, 1, 1], [2, 2], [3, 1], [4]]
> sage: b._list
> Traceback (most recent call last)
> ...
> AttributeError: 'Partitions_n_with_category' object has no attribute
'_list'
> }}}
> In this particular example this is not important because generating the
list of partitions of 4 is very quick, but if instead we were looking at
the partitions of 50 or 100 then this starts to become an issue.
>
> The reason, of course, is that ``Partitions`` is a subclass of
``UniqueRepresentation`` and ``order`` is an argument to the __init__
method of the class. I think that it would be preferable to make the
"ordered classes" proper subclasses of the ``Partitions`` class with the
default ordering. This way they would all be able to share their _list
attributes so that the list of all partitions in the class would only ever
need to be computed once. This could easily be done inside
`Partitions.__classcall_private__`.
I don't think direct subclasses will work since I don't see how the data
be actually communicated (without breaking into U.R.'s cache). Unless you
mean they are to act like proxy classes and have a link back to the common
data (which I believe would only consist of the list of partitions)?
Actually, this discussion is somewhat of a red herring since we need a
second list of all of the partitions because their parent would have
changed. The actual generation of the partitions I believe is amortized
`O(1)`, so at large scale, I would think other factors would start to
dominate the run time (ex. the memory allocation and manipulation, but I
could [easily] be wrong about this). Granted we can take advantage of the
fact that the partitions are suppose to be immutable, so we can share the
lists (I forget if I do this, but if I don't, I'll covert the internal
data to a tuple).
For example, suppose we had shared data, then we'd have the following
behavior:
{{{
sage: P = Partitions(50)
sage: Q = Partitions(50, order="dominance")
sage: Q._list[0].parent() == Q # The list would be generated with Q as the
parent since it is called from Q
True
sage: P._list[0].parent() == Q
True
}}}
>
> Another possibility would be to make ``order`` a part of
partition_options. In terms of the code this would be almost trivial as
currently ``order`` is used only inside the comparison methods.
Mathematically, I think that you can argue both ways: it is useful to be
able to use different orderings on the set of partitions so you don't want
an individual partition to be restricted to using a fixed order, on the
other the the poset of partitions with a particular order is a useful
structure.
>
> I think that Nicolas objected to doing it this way on the basis that it
might break code which implicitly assumed a particular order on the
partitions. I don't like this argument as it encourages bad coding: if a
particular ordering is required by the code then this should be explicit
in the code. (Up until now it hasn't been possible to easily change the
ordering being used, but now that it is becoming possible the algorithms
which require a particular ordering should be updated to make this
explicit.)
For example, every time you do `5 > 3`, you're assuming a particular order
on the integers. This is very similar to the issues with facade Posets
that were discussed about around 2 months (?) ago or so. Plus there are
some other subtleties that can arise (I can't recall them at the moment,
but I recall them having to do stored sage sessions).
>
> **I think that moving ``order`` into ``partition_options`` is the best
solution.**
>
> On the other hand, if you think it better to have honestly different
classes for each ordering then I think that the iterator should be
modified to produce the partitions in the correct order. This would be
annoying to do properly but could be done by first constructing the list
of all partitions and then sorting. This is potentially time consuming,
but if some one honestly needs a class for the poset with a particular
order then they probably need all of its elements(?).
I strongly don't think having a subclass with the desired order is a good
idea. Way too much maintenance and the problems above would likely still
exist. That's all the time I have right now to reply with. Again, thank
you for reviewing this!
Best,[[BR]]
Travis
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/13605#comment:15>
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 post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.