Fixing order functions of Pynac for sage 4.8?
Jean-Pierre Flori
Not sure you all followed what happened at #9880, but I think the
patches provided there are finally nearing an acceptable status.
So it could be a good idea to try to release a new version of pynac
and get them included in Sage 4.8.0 as it seems it will be the next
version of Sage.
AFAIK there are "only" two problems left to solve:
- the matching strange bug mentioned in my last comment,
- the doctests (i could ague that it is not really a big problem...).
I know everybody here is surely overbooked, but it would great to
finally finish that ticket which has been sitting on Trac for more
than one year, especially now that the last changes to make are kind
of easy in comparison with the initial work.
Plus, it would finally solve the different segfault mentioned on sage-
[support|devel] appearing now and then.
Bests,
JP
Burcin Erocal
On Sun, 6 Nov 2011 04:15:47 -0800 (PST)
Jean-Pierre Flori <jpf...@gmail.com> wrote:
> Not sure you all followed what happened at #9880, but I think the
> patches provided there are finally nearing an acceptable status.
> So it could be a good idea to try to release a new version of pynac
> and get them included in Sage 4.8.0 as it seems it will be the next
> version of Sage.
I agree that this is long overdue. I was hoping to finish it at SD31 in
June, but it turned out to be more work than I thought.
I don't know if we can make it to 4.8.0. This is going to be a big
change. Depending on Jeroen's speed in preparing the alpha's, it is
possible that we miss the merge window. Nevertheless, it is worth
trying.
I will try to find some time to look at the problem with match(). This
might be only next week though.
Thank you.
Cheers,
Burcin
Jean-Pierre Flori
I finally took some time to have a look at your last patches and have
some comments and questions.
I mainly concentrated on the buggy behavior of :
../sage/sage -c "var('x a'); print (-a+x)*(3*a-3*x)"
And the modifications to mul::get_sorted_seq() and
mul::do_print_rat_func(...)
My first question is about the code around Line 1507 in
get_sorted_seq() where you test for the leading minus and multiply the
add object by minus 1 if there is such a leading minus.
In the test you include a condition about cit->coeff being one or
minus one, i.e. the power the add object is elevated to.
Why such a limitation ?
I tried to get rid of it and consequently multiplying the overall
coeff by minus one only if the power was odd, but got into trouble:
- inconsistent behavior as before, the modified cit->rest does not
seem to be taken into account, even if the std::out << "rest after" <<
cit->rest does...
- and infinite recursion leading to segfault...
What I however observed is that if I change mul::recombine_pair_to_ex
(which is called in do_print_rat_func) to always return p.rest, and
never to try to build a power object, both the above problems are
solved.
Of course, I lose the exponents, but the minus sign of the overall
coeff is ok, and the factors are ok (i.e. I get -3*(a-x) for the above
example, if I change the exponents, the behavior is consistent)
So I guess there is something fishy going on when the power object is
generated.
Ill have a look at it later, maybe not before the next week-end
unfortunately.
Cheers.
On 7 nov, 10:58, Burcin Erocal <bur...@erocal.org> wrote:
> Hi Jean-Pierre,
>
> On Sun, 6 Nov 2011 04:15:47 -0800 (PST)
>
Burcin Erocal
On Sun, 27 Nov 2011 13:33:14 -0800 (PST)
Jean-Pierre Flori <jpf...@gmail.com> wrote:
> I finally took some time to have a look at your last patches and have
> some comments and questions.
> I mainly concentrated on the buggy behavior of :
> ../sage/sage -c "var('x a'); print (-a+x)*(3*a-3*x)"
> And the modifications to mul::get_sorted_seq() and
> mul::do_print_rat_func(...)
>
> My first question is about the code around Line 1507 in
> get_sorted_seq() where you test for the leading minus and multiply the
> add object by minus 1 if there is such a leading minus.
> In the test you include a condition about cit->coeff being one or
> minus one, i.e. the power the add object is elevated to.
> Why such a limitation ?
I wanted to revert the same rule from mul::eval() :
https://bitbucket.org/burcin/pynac/src/687b580c8c7c/ginac/mul.cpp#cl-676
> I tried to get rid of it and consequently multiplying the overall
> coeff by minus one only if the power was odd, but got into trouble:
> - inconsistent behavior as before, the modified cit->rest does not
> seem to be taken into account, even if the std::out << "rest after" <<
> cit->rest does...
> - and infinite recursion leading to segfault...
> What I however observed is that if I change mul::recombine_pair_to_ex
> (which is called in do_print_rat_func) to always return p.rest, and
> never to try to build a power object, both the above problems are
> solved.
> Of course, I lose the exponents, but the minus sign of the overall
> coeff is ok, and the factors are ok (i.e. I get -3*(a-x) for the above
> example, if I change the exponents, the behavior is consistent)
> So I guess there is something fishy going on when the power object is
> generated.
This may be the culprit:
https://bitbucket.org/burcin/pynac/src/687b580c8c7c/ginac/power.cpp#cl-617
In general, I'd like it if we can resolve this random printing issues
with minimum intervention in the existing code base. It would also be
great to avoid depending on the printing order for any basic arithmetic
operation. I can imagine using ginac to represent objects from
different domains in the future and defining different orderings to
make them print in a suitable way.
I am not sure if this minimum intervention wish is realistic.
Here is what I remember from my last pynac adventure:
- With the new order code, printing units is broken. They are shown
before the expression.
sage: sage.symbolic.units.convert(50 * x * units.area.square_meter,
units.area.acre)
acre*(1953125/158080329*x)
I don't mind this so much. :)
- If the operand access from Sage always uses the printing order,
expression_conversions.py ends up with infinite recursions.
An expression like 1/(x-y) might be represented as a mul with
overall coefficient -1 and a single sequence element, (-x+y)^(-1).
When you ask for nops(), you get 2. op(0) gives 1/(x-y) and op(1)
gives 1. In this case, code in expression_conversions just keeps
calling the conversion routine over the same expression.
The purpose of mul_to_power.patch was to prevent this by converting
such a mul object to a power in the first place. I don't consider
this a good solution. I wasn't thinking about the code in
power::eval() linked above when I wrote that.
Instead of the trick I pulled to change the function called from Sage
to "sorted_op", we should make this optional and use the internal
order in expression_conversions.py.
- calls to anything in normal.cpp produces random results. ATM, these
functions are not used much in Sage. But as Florent is writing
wrappers for numerator() and denominator(), we shouldn't ignore
them.
Random results are expected, since the factorization routines call
integer_content(), which returns with random sign. I can live with
normalizing the expressions w.r.t. the print order before a call to
gcd(), numerator(), denominator(), etc. from normal.cpp.
There is still some hope to switch to using Singular's factory to do
all the operations at some point anyway.
- The reasoning I mentioned above to pull out the leading -1 only when
the coeff (exponent in the mul representation) is 1 as a
counterpart to the code in mul::eval() sounds right to me. But it
doesn't work. We still print expressions like 1/(-x+1).
minus_denom.patch was intended to fix this case. I don't like this
solution either, but I didn't have time to investigate further.
That's all for now. Next week will be really busy for me. I will
probably spend the weekend preparing for that as well. I will try to
pick this up again towards the end of next week. Let's see how it goes.
Cheers,
Burcin