patch for SymPy <--> SAGE conversion, please review
Ondrej Certik
I fixed my bad patch, now it passes all tests in 2.8.14:
http://sagetrac.org/sage_trac/ticket/1189
the main patch (also linked from the ticket) in text form browsable over web:
http://sagetrac.org/sage_trac/attachment/ticket/1189/sympy2.patch
Could someone familiar with coerce.pyx review it please? It's actually
a very simple patch, but it could possibly cause troubles later
(mabshoff knows:).
Thanks a lot,
Ondrej
David Roe
David
William Stein
> Addition not commuting there bothers me. I can see why it's happening: a
> SymPy object doesn't call into the coercion system. One possible solution
> is to have coercion map sage objects into sympy, so both s+o and o+s (in
> line 134 and 135 of test_sympy.py) would be SymPy objects. Is SymPy and the
> SymbolicRing canonically isomorphic? Can every SymbolicRing element be cast
> into SymPy?
I don't think so, since probably sage/maxima have special functions
sympy doesn't. And even if it could be, the functions provided by
SymPy are different
than the ones provided by SymbolicRing.
If sympy('x') + x and x + sympy('x') are totally different in Sage I
will be unhappy about
that. Either the result has to always be in SymPy or always in Sage
or we have to get
an error and request that the user explicitly do a coercion.
We have to decide on a canonical map in exactly one direction, and I prefer
SymPy ---> SymbolicRing,
just because SymbolicRing is more native in Sage, and probably could be improved
so it would support any SymPy expression (yes -- I know this isn't
true now, unfortunately).
Now that I read through #1189 patch, I'm uncomfortable with
return _verify_canonical_coercion_c(x,y)
The function _verify_canonical_coercion_c is supposed to be like
assert(parent(x) is parent(y))
it's *never* supposed to be called when the parents are different -- if it is,
that means there is a bug. However, you're using it to test whether the
parents are different, and if so do some other behavior (i.e., eventually raise
a TypeError). So that makes me nervous. It would be better to just
directly do a call to the have_same_parent function, like in the implementation
of _verify_canonical_coercion_c in coerce.pxi.
In summary, the way the code is written now will work fine now, but seems
a little abusive.
David Roe
I don't think so, since probably sage/maxima have special functions
sympy doesn't. And even if it could be, the functions provided by
SymPy are different
than the ones provided by SymbolicRing.
If sympy('x') + x and x + sympy('x') are totally different in Sage I
will be unhappy about
that. Either the result has to always be in SymPy or always in Sage
or we have to get
an error and request that the user explicitly do a coercion.
We have to decide on a canonical map in exactly one direction, and I prefer
SymPy ---> SymbolicRing,
just because SymbolicRing is more native in Sage, and probably could be improved
so it would support any SymPy expression (yes -- I know this isn't
true now, unfortunately).
Now that I read through #1189 patch, I'm uncomfortable with
return _verify_canonical_coercion_c(x,y)
The function _verify_canonical_coercion_c is supposed to be like
assert(parent(x) is parent(y))
it's *never* supposed to be called when the parents are different -- if it is,
that means there is a bug. However, you're using it to test whether the
parents are different, and if so do some other behavior (i.e., eventually raise
a TypeError). So that makes me nervous. It would be better to just
directly do a call to the have_same_parent function, like in the implementation
of _verify_canonical_coercion_c in coerce.pxi.
Agreed.
David
Ondrej Certik
>
> On Nov 25, 2007 6:33 PM, David Roe <roe...@gmail.com> wrote:
> > Addition not commuting there bothers me. I can see why it's happening: a
> > SymPy object doesn't call into the coercion system. One possible solution
> > is to have coercion map sage objects into sympy, so both s+o and o+s (in
> > line 134 and 135 of test_sympy.py) would be SymPy objects. Is SymPy and the
> > SymbolicRing canonically isomorphic? Can every SymbolicRing element be cast
> > into SymPy?
>
> I don't think so, since probably sage/maxima have special functions
> sympy doesn't. And even if it could be, the functions provided by
> SymPy are different
> than the ones provided by SymbolicRing.
>
> If sympy('x') + x and x + sympy('x') are totally different in Sage I
> will be unhappy about
> that. Either the result has to always be in SymPy or always in Sage
> or we have to get
> an error and request that the user explicitly do a coercion.
>
> We have to decide on a canonical map in exactly one direction, and I prefer
>
> SymPy ---> SymbolicRing,
>
> just because SymbolicRing is more native in Sage, and probably could be improved
> so it would support any SymPy expression (yes -- I know this isn't
> true now, unfortunately).
David Roe:
> I would prefer that direction too. In order to make that happen, the SymPy
> __add__ function has to recognize a sage element and call its __add__ method
> instead. This sounds like it should be doable.
I am not sure it's a clean solution. Ideally, neither SAGE nor SymPy
should know about each other - of course both of them need to
implement the
_sage_ and _sympy_ methods (that of course need to know about the
other), but that should be it. Then you could plug any other library
in SAGE, not just SymPy and it will work. If you look at the patch
above:
373 elif isinstance(x, sympy.Basic):
374 #elif hasattr(x, "_sage_"):
375 # if "x" implements the _sage_() method, let's use
it to convert
376 # the expression to a SAGE expression (for example
SymPy provides
377 # _sage_() methods, some other libraries can too)
378 #
379 # unfortunately, the hasattr() above doesn't work (infinite
380 # recursion), so we check for SymPy directly.
381
382 return self(x._sage_())
It should work just by checking for the _sage_ method, not trying if
it is a SymPy object. Currently it doesn't, I get an infinite
recursion, so it's commented out (but I consider this a bug).
as to sympy('x') + SR(x) and SR(x) + sympy('x'):
it can be more complicated, like this:
SR(x) + SR(y) + sympy(x) and sympy(x)+sympy(y)+SR(x)
so you propose, that whenever there is some SAGE expression inside the
sympy expression, the result should be a SAGE expression? That seems
reasonable, but how about this:
sympy.sin(SR(x))
should it be a SAGE expression? How about sympy functions that are not
in SAGE/Maxima? This assumes that anything in SymPy can be converted
to SAGE, but that's not true, for example the real spherical harmonics
(there only seem to be complex harmonics in Maxima [1]), lucas
numbers, harmonic numbers, dirichlet eta function and any other
function, that the user creates himself, see [2] how to do it - he
just provides a simple class and then it works inside the SymPy
calculus natively. So I think the user would like to accept SAGE
expression inside his class and use it (if sympy supports all the
functions that are in the SAGE expression). With the current
behaviour, this will be possible.
One option would be to leave the current behaviour but redefine
commutative operations in SymPy to return SAGE expression if possible,
but I don't think it's a very clean solution either. BTW, this can be
done for example by patching the SymPy classes on the fly, or by some
other technique, so definitely there are technical means how to do it.
So I think for now, we should just convert to that expression, that
started the statement. i.e sympy(x)+SR(x) will convert to sympy and
SR(x)+sympy(x) will convert to SAGE. The user can always use SR() and
sympify() to convert to SAGE/SymPy explicitely. So we start with the
current behavior, which is imho a progress from the old behavior, and
then improve it iteratively.
> Now that I read through #1189 patch, I'm uncomfortable with
>
> return _verify_canonical_coercion_c(x,y)
>
> The function _verify_canonical_coercion_c is supposed to be like
>
> assert(parent(x) is parent(y))
>
> it's *never* supposed to be called when the parents are different -- if it is,
> that means there is a bug. However, you're using it to test whether the
> parents are different, and if so do some other behavior (i.e., eventually raise
> a TypeError). So that makes me nervous. It would be better to just
> directly do a call to the have_same_parent function, like in the implementation
> of _verify_canonical_coercion_c in coerce.pxi.
>
> In summary, the way the code is written now will work fine now, but seems
> a little abusive.
I think that code is just asking for troubles, but I didn't know how
to do it better. So you propose to do just this?
330 if PY_TYPE_CHECK(xp, type) or PY_TYPE_CHECK(yp, type):
331 if hasattr(x, "_sage_") and hasattr(y, "_sage_") and
have_same_parent(x,y) :
332 x = x._sage_()
333 y = y._sage_()
335 return _verify_canonical_coercion_c(x,y)
(The exception TypeError will be raised just a line later.)
Ondrej
[1] http://maxima.sourceforge.net/docs/manual/en/maxima_65.html
[2] http://code.google.com/p/sympy/wiki/SymPySvn
Joel B. Mohler
> David Roe:
> > I would prefer that direction too. In order to make that happen, the
> > SymPy __add__ function has to recognize a sage element and call its
> > __add__ method instead. This sounds like it should be doable.
>
> I am not sure it's a clean solution. Ideally, neither SAGE nor SymPy
> should know about each other - of course both of them need to
> implement the
> _sage_ and _sympy_ methods (that of course need to know about the
> other), but that should be it.
I don't agree with this -- see below for my reasoning.
I want results of operations with sage objects to be sage objects. This is
the same as it is with python objects now:
sage_int+python_int=python_int+sage_int=sage_int
Very simply, this is because I'm a sage user not a sympy user. I think that
the sage SymbolicExpressionRing needs to have the extra functionality to
properly wrap (i.e. in a sage-ish way) the sympy functionality. If we want
to use sympy as a back-end to implement these things (much like we do with
maxima), that's great. I realize that this is probably a longer term goal.
If we need a sub-optimal fix along the journey to get there, that may be
fine.
Sorry Ondrej, I certainly don't mean this as a strike against sympy. I think
it's a great project and I want sage and sympy to co-exist. However, one of
the beautiful things about sage (to me at least) is the pristine consistent
object hierarchy which neatly masks the many pieces behind it. I don't want
that compromised.
(In reality, I think that sympy and sage should simply merge. However, I
don't know enough about sympy to know how feasible that is. I put this in
parenthesis, because I fear it's kind of a demeaning thing to say. I don't
mean it that way though. It just feels like they are natural complements of
each other. The whole would be greater than the sum of it's parts.)
--
Joel
Ondrej Certik
> the same as it is with python objects now:
> sage_int+python_int=python_int+sage_int=sage_int
> Very simply, this is because I'm a sage user not a sympy user. I think that
> the sage SymbolicExpressionRing needs to have the extra functionality to
> properly wrap (i.e. in a sage-ish way) the sympy functionality. If we want
> to use sympy as a back-end to implement these things (much like we do with
> maxima), that's great. I realize that this is probably a longer term goal.
> If we need a sub-optimal fix along the journey to get there, that may be
> fine.
I agree with this, that working in SAGE should produce SAGE objects,
not sympy objects.
I am just thinking how to do it cleanly.
> Sorry Ondrej, I certainly don't mean this as a strike against sympy. I think
> it's a great project and I want sage and sympy to co-exist. However, one of
> the beautiful things about sage (to me at least) is the pristine consistent
> object hierarchy which neatly masks the many pieces behind it. I don't want
> that compromised.
>
> (In reality, I think that sympy and sage should simply merge. However, I
> don't know enough about sympy to know how feasible that is. I put this in
> parenthesis, because I fear it's kind of a demeaning thing to say. I don't
> mean it that way though. It just feels like they are natural complements of
> each other. The whole would be greater than the sum of it's parts.)
The problem with SAGE is that it is huge. And for many people it is
just too huge.
So finding ways to create SAGE more modular is the way to go imho. And
it will happen -
the notebook and maxima wrappers will eventually be released
separately. So I myself
definitely don't want to create something, so that users would need to
choose - either SAGE or
sympy. That would be very, very bad. But on the other hand, I want to
have a small library for calculus,
easy to use and easy to install and especially easy to extend. To give
you another application, right now we are discussing how to
do symbolic finite elements with sympy:
http://groups.google.com/group/sympy/browse_thread/thread/f73b00d1bdf0255a
using SAGE for it is not an option, as long as it is 150MB install.
However, the whole thing
can of course be included in SAGE.
And honestly, I don't think SAGE calculus can be based on sympy
anytime soon, it's just too slow. But I still
believe sympy is the way to go in the long term, otherwise I wouldn't
be doing it, right. :) So maxima should be
default in SAGE, while sympy being an option for people who want to
try it, if it works for them better than maxima.
BTW, I am still waiting for an answer to this:
the two examples at the end - those are things that I don't know how
to do easily in maxima.
Ondrej
Joel B. Mohler
> > (In reality, I think that sympy and sage should simply merge. However, I
> > don't know enough about sympy to know how feasible that is. I put this in
> > parenthesis, because I fear it's kind of a demeaning thing to say. I don't
> > mean it that way though. It just feels like they are natural complements of
> > each other. The whole would be greater than the sum of it's parts.)
>
> The problem with SAGE is that it is huge. And for many people it is
> just too huge.
> So finding ways to create SAGE more modular is the way to go imho. And
> it will happen -
> the notebook and maxima wrappers will eventually be released
> separately. So I myself
> definitely don't want to create something, so that users would need to
> choose - either SAGE or
> sympy. That would be very, very bad.
Well, if I had to pick a nasty point to sage I would agree that it's huge-ness
is seriously annoying. The slow import of "sage.all" really kills the
pleasure for writing python programs which you want to use from bash, but I
realize that I'm a bit unusual to actually use it that way. I would love
more modularity, but I'm not convinced it's possible. I mean, this huge-ness
is a wart in many ways, but quite frankly I've not seen any other mathematics
package which I would look forward to using for a lifetime of mathematics (at
all levels). I say that statement for mathematica as well as OSS
alternatives (just try writing a mathematica script to call from bash!). I
think the hugeness might just be an acceptable trade-off since making modular
software is much more difficult to make enjoyable to use.
I am wondering though what kind of support for matrices and polynomials (for
examples) you might envision with sympy. I see that you currently have
support listed for these things, but if it's pure python the sage equivalents
are going to be much faster. It seems that you could gain so much by sharing
these goals with sage.
On the flip side though, maxima is a gigantic ill-tempered beast to work with
and we really need a symbolic alternative -- sympy seems just the thing. I
think sage's symbolic side should be strong enough that we really shouldn't
have to break out the special polynomial declarations unless you are doing
something very special purpose. This might require a very very smart
symbolic engine to detect when it is working with polynomials and use
polynomial algorithms behind the scenes instead of more generic symbolic
ones. I think if we could pull that off, then even the number theorists
might find themselves working with the symbolic expressions. This would be a
huge step towards mathematica level friendliness imo.
I think I might be throwing this thread on a tangent though. Sorry.
--
Joel
Ondrej Certik
> is seriously annoying. The slow import of "sage.all" really kills the
> pleasure for writing python programs which you want to use from bash, but I
That's exactly how I want to write Python programs. And I am sure many
other users too.
And I think SAGE will eventually get there. The speed of import can be
cured, by importing
things on the fly, etc. etc.
> realize that I'm a bit unusual to actually use it that way. I would love
> more modularity, but I'm not convinced it's possible. I mean, this huge-ness
> is a wart in many ways, but quite frankly I've not seen any other mathematics
> package which I would look forward to using for a lifetime of mathematics (at
> all levels). I say that statement for mathematica as well as OSS
> alternatives (just try writing a mathematica script to call from bash!). I
> think the hugeness might just be an acceptable trade-off since making modular
> software is much more difficult to make enjoyable to use.
Agree.
> I am wondering though what kind of support for matrices and polynomials (for
> examples) you might envision with sympy. I see that you currently have
> support listed for these things, but if it's pure python the sage equivalents
> are going to be much faster. It seems that you could gain so much by sharing
> these goals with sage.
It has a huge advantage to have something lightweight in pure Python,
for many purposes.
And for many purposes, the speed is enough. For many purposes it
isn't. Because SAGE
has other good algorithms for polynomials, there's no point of using
the slow ones in sympy.
As to matrices, for pure numerics, one can use numpy, or some other
things that SAGE is using.
But for symbolic matrices - I think you can just use Maxima in SAGE.
As to gaining - it really depends on the application.
> On the flip side though, maxima is a gigantic ill-tempered beast to work with
> and we really need a symbolic alternative -- sympy seems just the thing. I
> think sage's symbolic side should be strong enough that we really shouldn't
> have to break out the special polynomial declarations unless you are doing
> something very special purpose. This might require a very very smart
> symbolic engine to detect when it is working with polynomials and use
> polynomial algorithms behind the scenes instead of more generic symbolic
> ones. I think if we could pull that off, then even the number theorists
> might find themselves working with the symbolic expressions. This would be a
> huge step towards mathematica level friendliness imo.
Agree.
Those are my goals and I think also of other developers of SAGE.
Ondrej
Robert Bradshaw
If the canonical map goes this direction, then the __add__ method of
SymPy objects needs to detect sage objects. I.e.
if isinstance(other, SageObject):
return self._sage_() + other
...
However, one cannot assume that SageObject is importable (nor does
one want to try and import it at every step, though this is much
faster than importing sage.all). I'm not sure the cleanest way to do
it. Perhaps there could be a method is_SageObject(x) that, at first
call, tries to import sage.structure.sage_object and on success
returns isinstance(x, SageObject) and on failure returns False. The
result of this import would be cached.
Alternatively, the root SageObject type could have an attribute that
one could look up to test its "sage-ness"
> Now that I read through #1189 patch, I'm uncomfortable with
>
> return _verify_canonical_coercion_c(x,y)
>
> The function _verify_canonical_coercion_c is supposed to be like
>
> assert(parent(x) is parent(y))
>
> it's *never* supposed to be called when the parents are different
> -- if it is,
> that means there is a bug. However, you're using it to test
> whether the
> parents are different, and if so do some other behavior (i.e.,
> eventually raise
> a TypeError). So that makes me nervous. It would be better to just
> directly do a call to the have_same_parent function, like in the
> implementation
> of _verify_canonical_coercion_c in coerce.pxi.
>
> In summary, the way the code is written now will work fine now, but
> seems
> a little abusive.
I think the code here should simply be
330 if PY_TYPE_CHECK(xp, type) or PY_TYPE_CHECK(yp,
type):
331 if hasattr(x, "_sage_") and hasattr(y, "_sage_"):
332 return canonical_coercion_c(x._sage_
(),y._sage_())
Ondrej Certik
That was my original patch, as seen here:
http://sagetrac.org/sage_trac/attachment/ticket/1189/sympy.patch
and it doesn't work, because sometimes SAGE puts "x=5" and
"y=something else" in, and expects to receive a TypeError, otherwise
it segfaults.
That's why the workaround.
Ondrej
Robert Bradshaw
I'm not seeing clearly what the problem is, could you please clarify
some more. Thanks.
- Robert
Ondrej Certik
> some more. Thanks.
If you apply my first patch, you will get these segfaults:
sage -t devel/sage-main/sage/schemes/generic/spec.py sh: line
1: 2816 Segmentation fault
/home/ondrej/ext/sage-2.8.13-x86_64-Linux/local/bin/python
.doctest_spec.py >.doctest/out 2>.doctest/err
A mysterious error (perphaps a memory error?) occurred, which may have
crashed doctest.
The problem, as it turns out, is on the line 97 in spec.py:
sage: Spec(QQ) == 5
False
which goes into my patch, with x==Spec(QQ) and y==5, it pass all my
ifs, and then it segfaults. The workaround is to check that they have
the same parent, or just call _verify_canonical_coercion_c, that does
exactly that for me.
Ondrej
William Stein
> Well, if I had to pick a nasty point to sage I would agree that it's huge-ness
> is seriously annoying. The slow import of "sage.all" really kills the
> pleasure for writing python programs which you want to use from bash, but I
> realize that I'm a bit unusual to actually use it that way. I would love
Ondrej says:
> And I think SAGE will eventually get there. The speed of import can be
> cured, by importing things on the fly, etc. etc.
Ondrej is 100% right.
This slow import is not necessarily a function of sage being huge, but of nobody
lately going through and auditing what happens when one does
$ sage -ipython
>>> import sage.all
I've done this in the past several times and each time greatly sped things up.
Doing this can be helpful:
$ echo "%prun import sage.all" |sage -ipython
Then commenting things out, etc., etc.
For example, importing numpy takes (or at least used to take) a surprisingly
long time, so I have to be careful to make sure Sage doesn't import that
by default on startup. Same goes for matplotlib -- never ever import that
at the module level -- make the import lazy, and things are much faster.
I think the sage startup time could be greatly reduced with some work.
> more modularity, but I'm not convinced it's possible. I mean, this huge-ness
> is a wart in many ways, but quite frankly I've not seen any other mathematics
> package which I would look forward to using for a lifetime of mathematics (at
When I'm actually doing real work, research, teaching, etc. this
hugeness is not a wart to me at least in any way at all. It means that
the resources are there to do what I want. Instead of hunting around for,
installing, building, learning new programs, I get work done. To quote
Michael Stoll when he switched from lots of little tools to Magma 10 years
ago: "it is very nice to have everything under one roof". He cares about
getting research done. Period. And could care less about "unix philosophy".
At the end of the day, I tend to agree.
> I am wondering though what kind of support for matrices and polynomials (for
> examples) you might envision with sympy. I see that you currently have
> support listed for these things, but if it's pure python the sage equivalents
> are going to be much faster. It seems that you could gain so much by sharing
> these goals with sage.
The sage equivalents only work when all coefficients of the polynomial lie
in a specific ring. Sympy does polynomial multiplication generically, which
requires generic files.
> and we really need a symbolic alternative -- sympy seems just the thing. I
> think sage's symbolic side should be strong enough that we really shouldn't
> have to break out the special polynomial declarations unless you are doing
> something very special purpose. This might require a very very smart
> symbolic engine to detect when it is working with polynomials and use
> polynomial algorithms behind the scenes instead of more generic symbolic
> ones. I think if we could pull that off, then even the number theorists
> might find themselves working with the symbolic expressions. This would be a
> huge step towards mathematica level friendliness imo.
You're right. In fact I already work with symbolic expressions
frequently, when it makes sense to do so. :-)
But personally my first priority is creating a viable alternative to
the big systems, before
trying to do some sort of open ended research system that's really
hard to write.
Ondrej:
> which goes into my patch, with x==Spec(QQ) and y==5, it pass all my
> ifs, and then it segfaults. The workaround is to check that they have
> the same parent, or just call _verify_canonical_coercion_c, that does
> exactly that for me.
It is wrong / abusive to call _verify_canonical_coercion_c because that
function is never supposed to fail. You should check that the parents
are the same explicitly and if not pass through to the next case.
William
Ondrej Certik
> >>> import sage.all
>
> I've done this in the past several times and each time greatly sped things up.
We did it too several times already in SymPy.
> When I'm actually doing real work, research, teaching, etc. this
> hugeness is not a wart to me at least in any way at all. It means that
> the resources are there to do what I want. Instead of hunting around for,
> installing, building, learning new programs, I get work done. To quote
> Michael Stoll when he switched from lots of little tools to Magma 10 years
> ago: "it is very nice to have everything under one roof". He cares about
> getting research done. Period. And could care less about "unix philosophy".
> At the end of the day, I tend to agree.
It really depends what you want to do. From your prospective (research
mathematics), this is exactly what you need.
From my prospective (using SAGE in my own programs), this isn't
exactly what I need, but fortnuately, this will improve,
when SAGE becomes more famous, and more people like Michael Abshoff
are going to join. I am looking forward to a time,
when I do in Debian:
$ apt-get install sage
$ python
Python 2.5.1 (r251:54863, Aug 17 2007, 00:51:07)
[GCC 4.1.3 20070812 (prerelease) (Debian 4.1.2-15)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from sage.all import *
>>> print x**2
And I could use anything that SAGE wraps. Everything under one hood. Period.
> > and we really need a symbolic alternative -- sympy seems just the thing. I
> > think sage's symbolic side should be strong enough that we really shouldn't
> > have to break out the special polynomial declarations unless you are doing
> > something very special purpose. This might require a very very smart
> > symbolic engine to detect when it is working with polynomials and use
> > polynomial algorithms behind the scenes instead of more generic symbolic
> > ones. I think if we could pull that off, then even the number theorists
> > might find themselves working with the symbolic expressions. This would be a
> > huge step towards mathematica level friendliness imo.
>
> You're right. In fact I already work with symbolic expressions
> frequently, when it makes sense to do so. :-)
>
> But personally my first priority is creating a viable alternative to
> the big systems, before
> trying to do some sort of open ended research system that's really
> hard to write.
Agree.
>
> It is wrong / abusive to call _verify_canonical_coercion_c because that
> function is never supposed to fail. You should check that the parents
> are the same explicitly and if not pass through to the next case.
Agree. That's why I am proposing the fix here (at the end of the message):
http://groups.google.com/group/sage-devel/msg/752742fd202f6868
as opposed to Robert's proposal here:
http://groups.google.com/group/sage-devel/msg/d0aab9c309a3aa29
So if there are no more objections (Robert?) I'll implement my
proposal and the patch will be ready.
Ondrej
William Stein
> exactly what I need, but fortnuately, this will improve,
> when SAGE becomes more famous, and more people like Michael Abshoff
> are going to join. I am looking forward to a time,
> when I do in Debian:
>
> $ apt-get install sage
> $ python
> Python 2.5.1 (r251:54863, Aug 17 2007, 00:51:07)
> [GCC 4.1.3 20070812 (prerelease) (Debian 4.1.2-15)] on linux2
> Type "help", "copyright", "credits" or "license" for more information.
> >>> from sage.all import *
> >>> print x**2
>
> And I could use anything that SAGE wraps. Everything under one hood. Period.
Indeed! This will be even better in the direction I like. The key
thing you (=Ondrej)
want really is that it is all under *your* existing roof (i.e., Debian) too.
Good.
William
Robert Bradshaw
>> It is wrong / abusive to call _verify_canonical_coercion_c because
>> that
>> function is never supposed to fail. You should check that the
>> parents
>> are the same explicitly and if not pass through to the next case.
>
> Agree. That's why I am proposing the fix here (at the end of the
> message):
>
> http://groups.google.com/group/sage-devel/msg/752742fd202f6868
>
> as opposed to Robert's proposal here:
>
> http://groups.google.com/group/sage-devel/msg/d0aab9c309a3aa29
>
> So if there are no more objections (Robert?) I'll implement my
> proposal and the patch will be ready.
I did a sage -upgrade and am waiting for it to finish building...
When that finishes I'm going to check out your segfault with Spec(QQ)
== 5. (If it doesn't finish soon I'll be on my way to campus, but
will get back to you tonight at the latest) This should not segfault,
but if it does I think it is due to an error elsewhere. Also, I think
we should only be callling _sage_ if one of the objects is not a
SageObject, rather than if one of the objects is not an Element. The
requirement would be that _sage_ MUST return an instance of
SageObject to avoid infinite recursion.
I still think _verify_canonical_coercion_c is the wrong thing to use
here, there is no reason that x._sage_() and y._sage_() should have
the same parent (e.g. one could be an element of SR, the other a sage
Integer), and at this point we should be able to succeed even if they
don't have the same parent by (re-)calling the Sage coercion model on
the Sage elements.
- Robert
Ondrej Certik
> > $ python
> > Python 2.5.1 (r251:54863, Aug 17 2007, 00:51:07)
> > [GCC 4.1.3 20070812 (prerelease) (Debian 4.1.2-15)] on linux2
> > Type "help", "copyright", "credits" or "license" for more information.
> > >>> from sage.all import *
> > >>> print x**2
> >
> > And I could use anything that SAGE wraps. Everything under one hood. Period.
>
> thing you (=Ondrej)
> want really is that it is all under *your* existing roof (i.e., Debian) too.
Yes, that's true that all I really care about is Debian on my
computers. But fortunately by the spirit of Debian,
if it works nicely in there, it's (usually) not a big problem to get
it work elsewhere, the same nicely.
> I did a sage -upgrade and am waiting for it to finish building...
> When that finishes I'm going to check out your segfault with Spec(QQ)
> == 5. (If it doesn't finish soon I'll be on my way to campus, but
> will get back to you tonight at the latest) This should not segfault,
> but if it does I think it is due to an error elsewhere. Also, I think
> we should only be callling _sage_ if one of the objects is not a
> SageObject, rather than if one of the objects is not an Element. The
> requirement would be that _sage_ MUST return an instance of
> SageObject to avoid infinite recursion.
>
> I still think _verify_canonical_coercion_c is the wrong thing to use
> here, there is no reason that x._sage_() and y._sage_() should have
> the same parent (e.g. one could be an element of SR, the other a sage
> Integer), and at this point we should be able to succeed even if they
> don't have the same parent by (re-)calling the Sage coercion model on
> the Sage elements.
Yes, I also think it's a bug somewhere else in SAGE. If you are able
to figure it out,
I am curious about it. Try my first patch, that will segfault. My
second patch passes fine.
Ondrej
Robert Bradshaw
>>
>> I still think _verify_canonical_coercion_c is the wrong thing to use
>> here, there is no reason that x._sage_() and y._sage_() should have
>> the same parent (e.g. one could be an element of SR, the other a sage
>> Integer), and at this point we should be able to succeed even if they
>> don't have the same parent by (re-)calling the Sage coercion model on
>> the Sage elements.
>
> Yes, I also think it's a bug somewhere else in SAGE. If you are able
> to figure it out,
> I am curious about it. Try my first patch, that will segfault. My
> second patch passes fine.
See
http://sagetrac.org/sage_trac/attachment/ticket/1189/sympy-coerce.patch
This on top of 2.8.14 + sympy.patch works great. (I wasn't able to
cleanly apply sympy2.patch which looks like it had some useful non-
coercion stuff in there as well.) I'm curious about your infinite
recursion bug for looking up the _sage_ attribute...
- Robert
William Stein
This is getting discussed to death but wouldn't it make more sense
to replace:
if not PY_TYPE_CHECK(x, SageObject) or not PY_TYPE_CHECK(y, SageObject):
x = x._sage_()
y = y._sage_()
return self.canonical_coercion_c(x, y)
by
cdef bint tx = PY_TYPE_CHECK(x, SageObject)
cdef bint ty = PY_TYPE_CHECK(y, SageObject)
if not tx or not ty:
x = x if tx else x._sage_()
y = y if ty else x._sage_()
return self.canonical_coercion_c(x, y)
Since otherwise you're very likely calling _sage_() on a sage object (in your
version of the code).
Robert Bradshaw
>>
>> See
>>
>> http://sagetrac.org/sage_trac/attachment/ticket/1189/sympy-
>> coerce.patch
>>
>> This on top of 2.8.14 + sympy.patch works great. (I wasn't able to
>> cleanly apply sympy2.patch which looks like it had some useful non-
>> coercion stuff in there as well.) I'm curious about your infinite
>> recursion bug for looking up the _sage_ attribute...
>
> This is getting discussed to death but wouldn't it make more sense
> to replace:
>
> if not PY_TYPE_CHECK(x, SageObject) or not PY_TYPE_CHECK(y,
> SageObject):
> x = x._sage_()
> y = y._sage_()
> return self.canonical_coercion_c(x, y)
>
> by
>
> cdef bint tx = PY_TYPE_CHECK(x, SageObject)
> cdef bint ty = PY_TYPE_CHECK(y, SageObject)
>
> if not tx or not ty:
> x = x if tx else x._sage_()
> y = y if ty else x._sage_()
> return self.canonical_coercion_c(x, y)
>
> Since otherwise you're very likely calling _sage_() on a sage
> object (in your
> version of the code).
Yes, this would be a bit faster, and could even be optimized further
knowing that exactly one of x and y is a non-sage object. However,
for sage objects the _sage_ method is fairly fast (return self,
though it is a python call), probably dwarfed by the other's _sage_
method.
Ondrej Certik
Thanks for your patch. So I'll implement the revised William's
version. I'll have a bier
once this "ondrej's #broken" get's accepted.
Ondra