doctests in category theory

13 views
Skip to first unread message

krastano...@gmail.com

unread,
Jun 19, 2012, 4:34:56 AM6/19/12
to sy...@googlegroups.com
There are some doctests in category theory that rely on the order of
printing of tests and fail when the ordering changes.
http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYh9MbDA

I had some similar issues with the ODE solvers.

Any general way to address such issues?

Aaron Meurer

unread,
Jun 19, 2012, 4:44:27 AM6/19/12
to sy...@googlegroups.com
We used to have this problem a lot, but then we made all printing
ordering canonical with sort_key. I suppose you should make sure that
the new category theory class printers use this.

Where did it come up in the ODE solvers?

Aaron Meurer

On Jun 19, 2012, at 2:35 AM, "krastano...@gmail.com"
> --
> You received this message because you are subscribed to the Google Groups "sympy" group.
> To post to this group, send email to sy...@googlegroups.com.
> To unsubscribe from this group, send email to sympy+un...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/sympy?hl=en.
>

Tom Bachmann

unread,
Jun 19, 2012, 4:51:14 AM6/19/12
to sy...@googlegroups.com
The problem seems to rather be that FiniteSet does not print canonically.

krastano...@gmail.com

unread,
Jun 19, 2012, 4:57:26 AM6/19/12
to sy...@googlegroups.com
When I was speaking about the ode solver it was a bit more general:
not knowing whether it is C1*sin(t) + C2*cos(t) or the permutation of
the constants. And I was speaking about the stuff that I added (that
is still in a pull request https://github.com/sympy/sympy/pull/1322),
so I may have caused errors.

Sergiu Ivanov

unread,
Jun 19, 2012, 6:41:29 PM6/19/12
to sy...@googlegroups.com
On Tue, Jun 19, 2012 at 11:51 AM, Tom Bachmann <e_m...@web.de> wrote:
> The problem seems to rather be that FiniteSet does not print canonically.

Indeed, those failures are related to how FiniteSet orders its
elements.

> On 19.06.2012 09:44, Aaron Meurer wrote:
>>
>> We used to have this problem a lot, but then we made all printing
>> ordering canonical with sort_key. I suppose you should make sure that
>> the new category theory class printers use this.

From what I can see in FiniteSet.__new__, FiniteSet always sorts its
elements with element_sort_fn which is defined just above the
definition of FiniteSet:

def element_sort_fn(x):
try:
if x.is_comparable is True:
return x
except:
pass
return 1e9+abs(hash(x))

From what I can see in StrPrinter._print_FiniteSet, no sort_key
function is involved when printing the elements of a FiniteSet.

Apparently, since is_comparable is not set for the objects of the
classes in sympy.categories.baseclasses, element_sort_fn(x) falls back
to the last line, which, I guess, is somewhat architecture and
a-lot-of-stuff dependent (at least because of the large float 1e9).

Sergiu

Matthew Rocklin

unread,
Jun 19, 2012, 9:35:17 PM6/19/12
to sy...@googlegroups.com
Right. FiniteSet's sorting was designed so that 
 - everything that knows how to sort itself does so
 - everything that does not know how to sort itself comes last and is sorted arbitrarily

hash will be machine dependent and so yes, this causes difficulties. 

I wasn't aware of default_sort_key while making FiniteSet. 
It is possible that it should be used instead of element_sort_fn.

Sergiu Ivanov

unread,
Jun 20, 2012, 10:15:55 AM6/20/12
to sy...@googlegroups.com
On Wed, Jun 20, 2012 at 4:35 AM, Matthew Rocklin <mroc...@gmail.com> wrote:
> Right. FiniteSet's sorting was designed so that
>  - everything that knows how to sort itself does so
>  - everything that does not know how to sort itself comes last and is sorted
> arbitrarily

Sounds reasonable.

> hash will be machine dependent and so yes, this causes difficulties.

Indeed.

> I wasn't aware of default_sort_key while making FiniteSet.
> It is possible that it should be used instead of element_sort_fn.

https://github.com/sympy/sympy/pull/1362

element_sort_fn is still there, for those elements which definitely
don't know how to sort themselves. For an example, consider one of
the tests:

S = FiniteSet((1,2), Float, A, -5, x, 'eggs', x**2, Interval)

class Float and class Interval don't have sort_key, so element_sort_fn
is still needed to assure at least some sorting for such unusual
elements.

Sergiu

Aaron Meurer

unread,
Jun 20, 2012, 5:48:25 PM6/20/12
to sy...@googlegroups.com
Perhaps those should not be allowed. Neither Float nor Interval (the
classes) are instances of Basic. 'eggs' isn't either but this can be
converted to Symbol('eggs') which is.

There was a similar discussion about this with Tuple recently.

Aaron Meurer

Aaron Meurer

unread,
Jun 21, 2012, 4:35:53 AM6/21/12
to sy...@googlegroups.com
That's more related to the way constants are numbered by constantsimp
than the printer. I don't remember if any work was done to make that
not depend on the order of .args (i.e., hashes).

Aaron Meurer

Sergiu Ivanov

unread,
Jun 21, 2012, 8:44:14 AM6/21/12
to sy...@googlegroups.com
On Thu, Jun 21, 2012 at 12:48 AM, Aaron Meurer <asme...@gmail.com> wrote:
> On Jun 20, 2012, at 8:15 AM, Sergiu Ivanov <unlimite...@gmail.com> wrote:
>>
>>  S = FiniteSet((1,2), Float, A, -5, x, 'eggs', x**2, Interval)
>>
>
> Perhaps those should not be allowed. Neither Float nor Interval (the
> classes) are instances of Basic. 'eggs' isn't either but this can be
> converted to Symbol('eggs') which is.

I had a similar idea. However, keeping FiniteSet capable of storing
anything has so far been rather easy, from what I can see. On the
other hand, I guess that allowing FiniteSet to store something which
is not instance of Basic is a bit inconsistent.

Perhaps, more people should express their opinion on this issue?

In the meantime, check out

https://github.com/sympy/sympy/pull/1368

which fixes the recent FiniteSet-related failures in master.

Sergiu

krastano...@gmail.com

unread,
Jun 22, 2012, 4:39:55 AM6/22/12
to sy...@googlegroups.com

Chris Smith

unread,
Jun 22, 2012, 4:42:45 AM6/22/12
to sy...@googlegroups.com
I made a note in 1368 (I think that's the number).
Reply all
Reply to author
Forward
0 new messages