Dear Alexey, Many thanks for your detailed analysis. Probably I need to give a bit of explanation how the old throwing exceptions version of remove_dirac_ONE works. Of course, there are many cases in which throwing an exception is the only possible outcome, e.g. division by zero. The cases you quoted in your email (and numerous other exceptions in GiNaC) are of this sort. However, the old remove_dirac_ONE throws an exception also on a bit more delicate situation: if the expression does not look like (although it may be) a Clifford scalar due to a dummy summation. In this case an expansion of all dummy summations is called (which we do not want normally). Alternatives which I see are all disputable: * Leave it to a user to catch the exception and perform a dummy index expansion and re-try scalarisation; * Make all dummy summation expansion at the start of the method in all case. Regarding your comment on the duplicated code, I understand the concern on the library size. On the other hand the method with exceptions can be really faster on huge expressions since it aborts on the first problematic term. On the other hand to leave only algorithm based on the exceptions (which can occur on an admissible expression as well) may potentially produce a less portable code. I cannot make a decision here... Best wishes, Vladimir -- Vladimir V. Kisil http://www.maths.leeds.ac.uk/~kisilv/ Book: Geometry of Mobius Transformations http://goo.gl/EaG2Vu Software: Geometry of cycles http://moebinv.sourceforge.net/
On Thu, 23 May 2019 15:32:12 +0400, Alexey Sheplyakov <asheplyakov@YANDEX.ru> said:
ASh> Hi, ASh> 23.05.2019, 10:44, "Vladimir V. Kisil" ASh> <kisilv@maths.leeds.ac.uk>: >> Dear Alexey, >> >> I am attaching another iteration of the patch as >> suggested. The exception-free method is used internally for a >> better protection. ASh> Firstly making two almost identical copies of remove_dirac_ONE ASh> is definitely a bad idea. Two possible approaches are: ASh> a) wrap exception-throwing remove_dirac_ONE like this ASh> ex remove_dirac_ONE(const ex& e, bool& success, unsigned char ASh> rl, unsigned options) { ex ret; try { ASh> ret = remove_dirac_ONE(e, rl, options); } ASh> catch (...) { success = false; ASh> ret = _ex0; } return ret; } ASh> This way exceptions will propagate within ginac.dll itself ASh> only, and there's no code duplication. ASh> b) alternatively one could move the actual computation to ASh> non-throwing variant of remove_dirac_ONE, and make a wrapper ASh> which does throw exceptions: ASh> ex remove_dirac_ONE(const ex& e, unsigned char rl, unsigned ASh> options) { bool success = true; ex ret = ASh> remove_dirac_ONE(e, success, rl, options); if (!success) { ASh> // perhaps non-throwing variant should set an integer ASh> status instead of a bool flag, so a more specific exception can ASh> be thrown throw std::runtime_error("some meaningful ASh> error message here"); } return ret; } ASh> Secondly I don't quite get what's the point of replacing the ASh> throwing variant of remove_dirac_ONE, and throwing the very ASh> same exception manually: >> + bool success; + if (! ex_to<idx>(mu).is_dim_numeric()) >> throw(std::invalid_argument("clifford_to_lst(): index should have >> a numeric dimension")); unsigned int D = >> ex_to<numeric>(ex_to<idx>(mu).get_dim()).to_int(); @@ -1341,7 >> +1399,9 @@ lst clifford_to_lst(const ex & e, const ex & c, bool >> algebraic) || (! is_a<numeric>(pow(c.subs(mu == i, >> subs_options::no_pattern), 2)))) algebraic = false; lst V; - ex >> v0 = >> remove_dirac_ONE(canonicalize_clifford(e+clifford_prime(e)))/2; + >> ex v0 = >> remove_dirac_ONE(canonicalize_clifford(e+clifford_prime(e)), >> success)/2; + if (!success) + >> throw(std::invalid_argument("clifford_to_lst(): the argument is >> not a Clifford vector")); ASh> I.e. why can't we keep calling ASh> ex v0 = ASh> remove_dirac_ONE(canonicalize_clifford(e+clifford_prime(e)))/2; ASh> and have remove_dirac_ONE throw an appropriate exception? >> Previously remote_dirac_ONE() reported that an expression is not >> a Clifford scalar by throwing an exception. This produced >> crashes in Qt applications on Windows. ASh> Not really. Throwing exceptions across shared libraries (DLLs) ASh> on Windows is a little bit tricky, but it's certainly possible. ASh> The crucial point is to make all DLLs use the same C++ runtime ASh> (which implies the C++ runtime should be linked dynamically ASh> into all DLLs linked into the application). ASh> Also patching just remove_dirac_ONE is sort of pointless, since ASh> most GiNaC methods and functions throw exceptions too. ASh> Best regards, Alexey