On Sat, Jun 23, 2012 at 10:53 AM, Aaron Meurer <[email protected]> wrote:
> On Jun 23, 2012, at 1:41 AM, Sergiu Ivanov <[email protected]> wrote:
>
>> On Sat, Jun 23, 2012 at 2:07 AM, Aaron Meurer <[email protected]> wrote:
>>> Without getting into the discussion about Basic args, it should be
>>> possible to sort the args in a platform independent way. For example,
>>> with str or __name__ (str should work unless it's a class that hasn't
>>> overridden it, which will have the id in the string).
>>
>> Yes, indeed; however, that would make the impact of creating a
>> FiniteSet even more serious, because doing str often requires
>> traversing the expression tree.
>
> I mean just using it for non-Basic types.

Oh, I see, that sounds great.

>>> My concern right now is more about getting tests to pass with hash
>>> randomization than with architectural issues, so that we can release.
>>> So I'd gladly see some quick fixes to get the tests to pass now, and
>>> we can work out a better architecture later.
>>
>> I'll to try to drop the sorting right now and see how I can fix the
>> things that get broken (if something indeed gets so).
>
> Of course, if you can do this, it will be even better.

I'm coming back with not-that-good news.

FiniteSet creates a frozenset internally, to drop duplicates.  If we
just convert that to a list, we get essentially arbitrary order of
elements in the FiniteSet.  This is going to randomly break every
single doctest showing FiniteSet.  I guess this was the initial
thought behind sorting the elements of the FiniteSet.

There are two immediately available ways out:

1. Don't use frozenset to remove duplicates; instead, remove them by
   hand, preserving the order of the elements as they were supplied to
   the constructor.  This will result in O(1/2 n^2) complexity of
   creating FiniteSet, which is most probably worse than what it is at
   the moment.

2. Have FiniteSet sort its elements, just as it does now.  However,
   fix that one doctest which puts *two* non-Basic objects in a
   FiniteSet to only put *one* non-Basic object.  This will convey the
   same message and avoid the dependency on the machine.  Note that
   the failures in the doctests now are only due to the fact that the
   two non-Basic elements in one single doctests are getting sorted in
   different order with respect to each other, while all other
   elements always get arranged in the same way.

I'd vote for the second approach, because it's fastest and simplest.
Note that, apparently, there is only one doctest in the whole SymPy
which places two non-Basic objects in a FiniteSet and thus causes
trouble.  Also note that the problem we are currently facing only
appears in the doctests.

This however does not solve the important questions raised in this
thread.  I'm still not sure whether it's good that FiniteSet is able
to store anything.  It's still not a good idea to actually sort things
in FiniteSet.  I guess I could open issues for the two, if no one
objects.

Sergiu

-- 
You received this message because you are subscribed to the Google Groups 
"sympy" 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/sympy?hl=en.

Reply via email to