[PATCH 1/8] inifcns_nstdsums.cpp: S_num takes cl_N as an argument instead of numeric.
Implicit conversion from cl_N to numeric considered harmful. Using GiNaC::numeric for numerical computations incurs significant overhead, so one might want to do these computations using proper CLN types. Unfortunately, it's not easy due to automatic conversion from cln::cl_N to GiNaC::numeric. Here is a simple example: cl_N x, y; // initialize them return sin(x) + y*exp(y); The compiler complains about ambigously overloaded of functions, i.e. cl_N cln::sin(const cl_N&) versus numeric GiNaC::sin(const numeric&). Hence, I propose to disable *implicit* conversion from cl_N to numeric (this can be done by marking the numeric ctor as `explicit'). However, this change happens to be a bit nontrivial, because that evil conversion is used in GiNaC itself. So, I decided to rewrite those fragments of code. --- ginac/inifcns_nstdsums.cpp | 50 ++++++++++++++++++++++++++----------------- 1 files changed, 30 insertions(+), 20 deletions(-) diff --git a/ginac/inifcns_nstdsums.cpp b/ginac/inifcns_nstdsums.cpp index cd3511a..ea42a6e 100644 --- a/ginac/inifcns_nstdsums.cpp +++ b/ginac/inifcns_nstdsums.cpp @@ -320,7 +320,7 @@ cln::cl_N Lin_do_sum_Xn(int n, const cln::cl_N& x) // forward declaration needed by function Li_projection and C below -numeric S_num(int n, int p, const numeric& x); +const cln::cl_N S_num(int n, int p, const cln::cl_N& x); // helper function for classical polylog Li @@ -371,7 +371,7 @@ cln::cl_N Li_projection(int n, const cln::cl_N& x, const cln::float_format_t& pr } else { cln::cl_N result = -cln::expt(cln::log(x), n-1) * cln::log(1-x) / cln::factorial(n-1); for (int j=0; j<n-1; j++) { - result = result + (S_num(n-j-1, 1, 1).to_cl_N() - S_num(1, n-j-1, 1-x).to_cl_N()) + result = result + (S_num(n-j-1, 1, 1) - S_num(1, n-j-1, 1-x)) * cln::expt(cln::log(x), j) / cln::factorial(j); } return result; @@ -402,7 +402,7 @@ numeric Lin_numeric(int n, const numeric& x) cln::cl_N x_ = ex_to<numeric>(x).to_cl_N(); cln::cl_N result = -cln::expt(cln::log(x_), n-1) * cln::log(1-x_) / cln::factorial(n-1); for (int j=0; j<n-1; j++) { - result = result + (S_num(n-j-1, 1, 1).to_cl_N() - S_num(1, n-j-1, 1-x_).to_cl_N()) + result = result + (S_num(n-j-1, 1, 1) - S_num(1, n-j-1, 1-x_)) * cln::expt(cln::log(x_), j) / cln::factorial(j); } return result; @@ -1715,10 +1715,10 @@ cln::cl_N C(int n, int p) if (k == 0) { if (n & 1) { if (j & 1) { - result = result - 2 * cln::expt(cln::pi(),2*j) * S_num(n-2*j,p,1).to_cl_N() / cln::factorial(2*j); + result = result - 2 * cln::expt(cln::pi(),2*j) * S_num(n-2*j,p,1) / cln::factorial(2*j); } else { - result = result + 2 * cln::expt(cln::pi(),2*j) * S_num(n-2*j,p,1).to_cl_N() / cln::factorial(2*j); + result = result + 2 * cln::expt(cln::pi(),2*j) * S_num(n-2*j,p,1) / cln::factorial(2*j); } } } @@ -1726,23 +1726,23 @@ cln::cl_N C(int n, int p) if (k & 1) { if (j & 1) { result = result + cln::factorial(n+k-1) - * cln::expt(cln::pi(),2*j) * S_num(n+k-2*j,p-k,1).to_cl_N() + * cln::expt(cln::pi(),2*j) * S_num(n+k-2*j,p-k,1) / (cln::factorial(k) * cln::factorial(n-1) * cln::factorial(2*j)); } else { result = result - cln::factorial(n+k-1) - * cln::expt(cln::pi(),2*j) * S_num(n+k-2*j,p-k,1).to_cl_N() + * cln::expt(cln::pi(),2*j) * S_num(n+k-2*j,p-k,1) / (cln::factorial(k) * cln::factorial(n-1) * cln::factorial(2*j)); } } else { if (j & 1) { - result = result - cln::factorial(n+k-1) * cln::expt(cln::pi(),2*j) * S_num(n+k-2*j,p-k,1).to_cl_N() + result = result - cln::factorial(n+k-1) * cln::expt(cln::pi(),2*j) * S_num(n+k-2*j,p-k,1) / (cln::factorial(k) * cln::factorial(n-1) * cln::factorial(2*j)); } else { result = result + cln::factorial(n+k-1) - * cln::expt(cln::pi(),2*j) * S_num(n+k-2*j,p-k,1).to_cl_N() + * cln::expt(cln::pi(),2*j) * S_num(n+k-2*j,p-k,1) / (cln::factorial(k) * cln::factorial(n-1) * cln::factorial(2*j)); } } @@ -1855,7 +1855,7 @@ cln::cl_N S_projection(int n, int p, const cln::cl_N& x, const cln::float_format res2 = res2 + cln::expt(cln::cl_I(-1),r) * cln::expt(cln::log(1-x),r) * S_do_sum(p-r,n-s,1-x,prec) / cln::factorial(r); } - result = result + cln::expt(cln::log(x),s) * (S_num(n-s,p,1).to_cl_N() - res2) / cln::factorial(s); + result = result + cln::expt(cln::log(x),s) * (S_num(n-s,p,1) - res2) / cln::factorial(s); } return result; @@ -1866,7 +1866,7 @@ cln::cl_N S_projection(int n, int p, const cln::cl_N& x, const cln::float_format // helper function for S(n,p,x) -numeric S_num(int n, int p, const numeric& x) +const cln::cl_N S_num(int n, int p, const cln::cl_N& x) { if (x == 1) { if (n == 1) { @@ -1902,11 +1902,11 @@ numeric S_num(int n, int p, const numeric& x) // what is the desired float format? // first guess: default format cln::float_format_t prec = cln::default_float_format; - const cln::cl_N value = x.to_cl_N(); + const cln::cl_N value = x; // second guess: the argument's format - if (!x.real().is_rational()) + if (!instanceof(realpart(value), cln::cl_RA_ring)) prec = cln::float_format(cln::the<cln::cl_F>(cln::realpart(value))); - else if (!x.imag().is_rational()) + else if (!instanceof(imagpart(value), cln::cl_RA_ring)) prec = cln::float_format(cln::the<cln::cl_F>(cln::imagpart(value))); // [Kol] (5.3) @@ -1919,9 +1919,9 @@ numeric S_num(int n, int p, const numeric& x) cln::cl_N res2; for (int r=0; r<p; r++) { res2 = res2 + cln::expt(cln::cl_I(-1),r) * cln::expt(cln::log(1-value),r) - * S_num(p-r,n-s,1-value).to_cl_N() / cln::factorial(r); + * S_num(p-r,n-s,1-value) / cln::factorial(r); } - result = result + cln::expt(cln::log(value),s) * (S_num(n-s,p,1).to_cl_N() - res2) / cln::factorial(s); + result = result + cln::expt(cln::log(value),s) * (S_num(n-s,p,1) - res2) / cln::factorial(s); } return result; @@ -1936,7 +1936,7 @@ numeric S_num(int n, int p, const numeric& x) for (int r=0; r<=s; r++) { result = result + cln::expt(cln::cl_I(-1),s) * cln::expt(cln::log(-value),r) * cln::factorial(n+s-r-1) / cln::factorial(r) / cln::factorial(s-r) / cln::factorial(n-1) - * S_num(n+s-r,p-s,cln::recip(value)).to_cl_N(); + * S_num(n+s-r,p-s,cln::recip(value)); } } result = result * cln::expt(cln::cl_I(-1),n); @@ -1972,12 +1972,18 @@ numeric S_num(int n, int p, const numeric& x) static ex S_evalf(const ex& n, const ex& p, const ex& x) { if (n.info(info_flags::posint) && p.info(info_flags::posint)) { + const int n_ = ex_to<numeric>(n).to_int(); + const int p_ = ex_to<numeric>(p).to_int(); if (is_a<numeric>(x)) { - return S_num(ex_to<numeric>(n).to_int(), ex_to<numeric>(p).to_int(), ex_to<numeric>(x)); + const cln::cl_N x_ = ex_to<numeric>(x).to_cl_N(); + const cln::cl_N result = S_num(n_, p_, x_); + return numeric(result); } else { ex x_val = x.evalf(); if (is_a<numeric>(x_val)) { - return S_num(ex_to<numeric>(n).to_int(), ex_to<numeric>(p).to_int(), ex_to<numeric>(x_val)); + const cln::cl_N x_val_ = ex_to<numeric>(x_val).to_cl_N(); + const cln::cl_N result = S_num(n_, p_, x_val_); + return numeric(result); } } } @@ -2002,7 +2008,11 @@ static ex S_eval(const ex& n, const ex& p, const ex& x) return Li(n+1, x); } if (x.info(info_flags::numeric) && (!x.info(info_flags::crational))) { - return S_num(ex_to<numeric>(n).to_int(), ex_to<numeric>(p).to_int(), ex_to<numeric>(x)); + int n_ = ex_to<numeric>(n).to_int(); + int p_ = ex_to<numeric>(p).to_int(); + const cln::cl_N x_ = ex_to<numeric>(x).to_cl_N(); + const cln::cl_N result = S_num(n_, p_, x_); + return numeric(result); } } if (n.is_zero()) { -- 1.5.4.2 -- All science is either physics or stamp collecting.
Dear Alexei, Alexei Sheplyakov wrote:
Implicit conversion from cl_N to numeric considered harmful.
I agree: It's quite a greedy mechanism that does have its pitfalls.
Using GiNaC::numeric for numerical computations incurs significant overhead, so one might want to do these computations using proper CLN types. Unfortunately, it's not easy due to automatic conversion from cln::cl_N to GiNaC::numeric. Here is a simple example:
cl_N x, y; // initialize them return sin(x) + y*exp(y);
The compiler complains about ambigously overloaded of functions, i.e. cl_N cln::sin(const cl_N&) versus numeric GiNaC::sin(const numeric&).
Does it? Can you provide a complete example? This program compiles fine: #include <cln/cln.h> #include <ginac/ginac.h> using namespace GiNaC; using namespace cln; int main() { cl_N x, y; // initialize them numeric result = sin(x) + y*exp(y); }
Hence, I propose to disable *implicit* conversion from cl_N to numeric (this can be done by marking the numeric ctor as `explicit').
Or, alternatively, by making it private! (But I'm not sure if this does eliminate all ambiguities.)
However, this change happens to be a bit nontrivial, because that evil conversion is used in GiNaC itself. So, I decided to rewrite those fragments of code.
Is there a reason you haven't committed it somewhere? I'm a beginner with git, but it would appear to me that this could be handled easily with a branch, right? Best wishes -richy. PS: I've pushed your other changes. -- Richard B. Kreckel <http://www.ginac.de/~kreckel/>
Richard B. Kreckel wrote:
Hence, I propose to disable *implicit* conversion from cl_N to numeric (this can be done by marking the numeric ctor as `explicit').
Or, alternatively, by making it private!
Oh, scratch this. Nobody wants to break third-party code that uses both GiNaC and CLN. -richy. -- Richard B. Kreckel <http://www.ginac.de/~kreckel/>
Dear Richard, On Sat, Mar 22, 2008 at 12:28:07AM +0100, Richard B. Kreckel wrote:
Using GiNaC::numeric for numerical computations incurs significant overhead, so one might want to do these computations using proper CLN types. Unfortunately, it's not easy due to automatic conversion from cln::cl_N to GiNaC::numeric. Here is a simple example:
cl_N x, y; // initialize them return sin(x) + y*exp(y);
The compiler complains about ambigously overloaded of functions, i.e. cl_N cln::sin(const cl_N&) versus numeric GiNaC::sin(const numeric&).
Does it? Can you provide a complete example?
$ cat numconv2.cpp #include <cln/cln.h> #include <ginac/ginac.h> #include <iostream> using namespace GiNaC; int main(int argc, char** argv) { symbol a("a"); cln::cl_N x = cln::cl_float(2), y = cln::pi()/6; ex e = a*(1 + log(x + y)*y + x*cos(y)); std::cout << e << std::endl; return 0; } $ g++ -Wall -O0 -g numconv2.cpp -lginac -lcln numconv2.cpp: In function ‘int main(int, char**)’: numconv2.cpp:10: error: no match for ‘operator*’ in ‘a * cln::operator+(const cln::cl_N&, const cln::cl_N&)(((const cln::cl_N&)(& cln::operator*(const cln::cl_ N&, const cln::cl_N&)(((const cln::cl_N&)(& cln::cos(const cln::cl_N&)()))))))’ /usr/include/ginac/operators.h:37: note: candidates are: const GiNaC::ex GiNaC::operator*(const GiNaC::ex&, const GiNaC::ex&) /usr/include/ginac/operators.h:43: note: const GiNaC::numeric GiNaC::operator*(const GiNaC::numeric&, const GiNaC::numeric&) /usr/include/cln/univpoly_modint.h:188: note: const cln::cl_UP_MI cln::operator*(const cln::cl_UP_MI&, const cln::cl_MI&) [skipped the rest] But my analysis seems to be wrong. Anyway, I'd like to get rid of implicit conversion, since it's very confusing.
Is there a reason you haven't committed it somewhere?
Yes. It's a good idea to discuss a patch before commiting it to a public branch.
I'm a beginner with git, but it would appear to me that this could be handled easily with a branch, right?
Actually, it is handled with a branch. But this brunch is not public (so I can rebase it, delete wrong commits, etc., without confusing people who cloned my repository). Best regards, Alexei -- All science is either physics or stamp collecting.
Dear Alexei, Alexei Sheplyakov wrote:
Using GiNaC::numeric for numerical computations incurs significant overhead, so one might want to do these computations using proper CLN types. Unfortunately, it's not easy due to automatic conversion from cln::cl_N to GiNaC::numeric. Here is a simple example:
cl_N x, y; // initialize them return sin(x) + y*exp(y);
The compiler complains about ambigously overloaded of functions, i.e. cl_N cln::sin(const cl_N&) versus numeric GiNaC::sin(const numeric&). Does it? Can you provide a complete example?
$ cat numconv2.cpp
#include <cln/cln.h> #include <ginac/ginac.h> #include <iostream> using namespace GiNaC;
int main(int argc, char** argv) { symbol a("a"); cln::cl_N x = cln::cl_float(2), y = cln::pi()/6; ex e = a*(1 + log(x + y)*y + x*cos(y)); std::cout << e << std::endl; return 0; }
$ g++ -Wall -O0 -g numconv2.cpp -lginac -lcln
numconv2.cpp: In function ‘int main(int, char**)’: numconv2.cpp:10: error: no match for ‘operator*’ in ‘a * cln::operator+(const cln::cl_N&, const cln::cl_N&)(((const cln::cl_N&)(& cln::operator*(const cln::cl_ N&, const cln::cl_N&)(((const cln::cl_N&)(& cln::cos(const cln::cl_N&)()))))))’ /usr/include/ginac/operators.h:37: note: candidates are: const GiNaC::ex GiNaC::operator*(const GiNaC::ex&, const GiNaC::ex&) /usr/include/ginac/operators.h:43: note: const GiNaC::numeric GiNaC::operator*(const GiNaC::numeric&, const GiNaC::numeric&) /usr/include/cln/univpoly_modint.h:188: note: const cln::cl_UP_MI cln::operator*(const cln::cl_UP_MI&, const cln::cl_MI&) [skipped the rest]
But my analysis seems to be wrong.
Well, the example is calling for an operator*(symbol, cl_N) and the compiler's only matches are operator*(ex, ex) and operator*(cl_<type1>, cl_<type2>). Class numeric isn't involved and making that ctor explicit won't help.
Anyway, I'd like to get rid of implicit conversion, since it's very confusing.
That conversion seems to be rarely used except inside GiNaC proper. At least Orsa, nestedsums, and xloops do not use it. I don't think anyone would miss its removal. -richy. -- Richard B. Kreckel <http://www.ginac.de/~kreckel/>
Hi, Back in March, Alexei posted a series of eight patches which made the conversion from cln::cl_N to GiNaC::numeric explicit. There were no objections to such a change. Alexei Sheplyakov wrote:
I'm a beginner with git, but it would appear to me that this could be handled easily with a branch, right?
Actually, it is handled with a branch. But this brunch is not public (so I can rebase it, delete wrong commits, etc., without confusing people who cloned my repository).
Can we arrange to merge this patch into the master branch? Cheers -richy. -- Richard B. Kreckel <http://www.ginac.de/~kreckel/>
Hi, Richard B. Kreckel schrieb:
Back in March, Alexei posted a series of eight patches which made the conversion from cln::cl_N to GiNaC::numeric explicit. There were no objections to such a change.
Can we arrange to merge this patch into the master branch?
I can/will do that this evening. Regards, Jens
participants (3)
-
Alexei Sheplyakov
-
Jens Vollinga
-
Richard B. Kreckel