remove_dirac_ONE() and documentation patches
Dear All, I am sending three patches for consideration. First two add some details to the GiNaC tutorial. The third one replaces remove_dirac_ONE() from the clifford.cpp. Currently remove_dirac_ONE() uses exceptions in normal operation to terminate a chain of recurrent calls. Some people would say that exceptions are evil, but this did not cause problems until I try to make a port for another evil---Windows. The exceptions in remove_dirac_ONE() terminated a Qt Windows application although the same code smoothly run under Linux. Thus I re-write remove_dirac_ONE() without exceptions. The signature of the method has to change, but a user may not notice this in the first approximation. Some respective alterations to the tutorial and the check suit are made as well. 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/
Hi,
Thus I re-write remove_dirac_ONE() without exceptions. The signature of the method has to change, but a user may not notice this in the first approximation.
There a few fatal differences (which are indeed not so easy to spot): - A user code might be relying on remove_dirac_ONE() to throw an exception when something goes wrong. The patch breaks such a code without a slightest warning from the compiler. - Before the patch this: remove_dirac_ONE(e, 0, 0); used to compile and work just fine. With the patch it also compiles (without a slightest warning), and segfaults at the run time (for the last 0 is `bool *success` now).0
* @param e Expression to be processed - * @param rl Value of representation label - * @param options Defines some internal use */ -ex remove_dirac_ONE(const ex & e, unsigned char rl = 0, unsigned options = 0); + * @param rl Value of representation label, all dirac_ONE with this or greater value will be processed + * @param success It is changed to false if there is at least one clifford_unit() in the expression + * @param options Defines some internal value for recursive calls, shall be ommited in user code */ +ex remove_dirac_ONE(const ex & e, unsigned char rl = 0, bool *success = new bool, unsigned options = 0);
Also there's a memory leak (since nobody is going to release that `new bool`). Therefore I think the patch is wrong and should not be applied. Possible solutions: 1) a) Make a new (overload of) remove_dirac_ONE function: ex remove_dirac_ONE(const ex& e, bool& success, unsigned char rl = 0, unsigned options = 0); which reports errors without exceptions b) Don't change the behavior of the existing `ex remove_dirac_ONE(const ex& e, unsigned char rl =0, unsigned options = 0); 2) Leave current `remove_dirac_ONE` as is. It's not broken, there's nothing to fix.
The exceptions in remove_dirac_ONE() terminated a Qt Windows application
And fix that application instead (perhaps I could help if the sources and instructions how to reproduce the problem are available) Best regards, Alexey
Dear Alexey, Many thanks for the useful suggestions. I have revised my patch in accordance with your first suggestion: both the old and the new version are present. This patch is attached below. Regarding crashes of my application the situation was solved by partial substitution: I was able to avoid crashes keeping the original version of GiNaC. It was enough to replace calls to exception-free version of remove_dirac_ONE to in my library only. Probably exceptions work fine within one library but create crashes if passed between different libraries. 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 Sun, 19 May 2019 19:31:06 +0400, Alexey Sheplyakov <asheplyakov@yandex.ru> said:
ASh> Hi, >> Thus I re-write remove_dirac_ONE() without exceptions. The >> signature of the method has to change, but a user may not >> notice this in the first approximation. ASh> There a few fatal differences (which are indeed not so easy to ASh> spot): ASh> - A user code might be relying on remove_dirac_ONE() to throw ASh> an exception when something goes wrong. The patch breaks such ASh> a code without a slightest warning from the compiler. ASh> - Before the patch this: ASh> remove_dirac_ONE(e, 0, 0); ASh> used to compile and work just fine. With the patch it also ASh> compiles (without a slightest warning), and segfaults at the ASh> run time (for the last 0 is `bool *success` now).0 >> * @param e Expression to be processed - * @param rl Value of >> representation label - * @param options Defines some internal use >> */ -ex remove_dirac_ONE(const ex & e, unsigned char rl = 0, >> unsigned options = 0); + * @param rl Value of representation >> label, all dirac_ONE with this or greater value will be processed >> + * @param success It is changed to false if there is at least >> one clifford_unit() in the expression + * @param options Defines >> some internal value for recursive calls, shall be ommited in user >> code */ +ex remove_dirac_ONE(const ex & e, unsigned char rl = 0, >> bool *success = new bool, unsigned options = 0); ASh> Also there's a memory leak (since nobody is going to release ASh> that `new bool`). ASh> Therefore I think the patch is wrong and should not be applied. ASh> Possible solutions: ASh> 1) ASh> a) Make a new (overload of) remove_dirac_ONE function: ASh> ex remove_dirac_ONE(const ex& e, bool& success, unsigned ASh> char rl = 0, unsigned options = 0); ASh> which reports errors without exceptions ASh> b) Don't change the behavior of the existing `ex ASh> remove_dirac_ONE(const ex& e, unsigned char rl =0, unsigned ASh> options = 0); ASh> 2) Leave current `remove_dirac_ONE` as is. It's not broken, ASh> there's nothing to fix. >> The exceptions in remove_dirac_ONE() terminated a Qt Windows >> application ASh> And fix that application instead (perhaps I could help if the ASh> sources and instructions how to reproduce the problem are ASh> available) ASh> Best regards, Alexey ASh> _______________________________________________ GiNaC-devel ASh> mailing list GiNaC-devel@ginac.de ASh> https://www.cebix.net/mailman/listinfo/ginac-devel
Hello, 20.05.2019, 02:29, "Vladimir V. Kisil" <kisilv@maths.leeds.ac.uk>:
Many thanks for the useful suggestions. I have revised my patch in accordance with your first suggestion: both the old and the new version are present. This patch is attached below.
This version looks better, but
+ex remove_dirac_ONE(const ex & e, bool *success, unsigned char rl = 0, unsigned options = 0); +
it still subtly changes the meaning of remove_dirac_ONE(e, 0, 0); Before the patch it was remove_dirac_ONE(e, /* rl = */ 0, /* options = */ 0), and with patch it's remove_dirac_ONE(e, /* success = */ 0, /* rl = */ 0) which 1) does not throw an exception on error 2) segfaults Please change the prototype (of the non-throwing function) to ex remove_dirac_ONE(const ex& e, bool& success, unsigned char rl = 0, unsigned options = 0); This way the compiler can pick the correct overload e = remove_dirac_ONE(e, 0, 0); // OK: remove_dirac_ONE(const ex&, unsigned char, unsigned) bool ok = false; e = remove_dirac_ONE(e, ok, 0, 0);
Regarding crashes of my application the situation was solved by partial substitution: I was able to avoid crashes keeping the original version of GiNaC. It was enough to replace calls to exception-free version of remove_dirac_ONE to in my library only. Probably exceptions work fine within one library but create crashes if passed between different libraries.
I don't quite understand how patching *just* remove_dirac_ONE could possibly avoid the cross-DLL exceptions problem. See, many (most?) GiNaC methods and functions use exceptions to report an error. For instance, pseries::power_const throws pole_error. Why this one is not a problem? What's special about remove_dirac_ONE? Best regards, Alexey
Dear Alexey, I am attaching another iteration of the patch as suggested. The exception-free method is used internally for a better protection. 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 Mon, 20 May 2019 18:06:38 +0400, Alexey Sheplyakov <asheplyakov@yandex.ru> said:
ASh> Hello, ASh> 20.05.2019, 02:29, "Vladimir V. Kisil" ASh> <kisilv@maths.leeds.ac.uk>: >> Many thanks for the useful suggestions. I have revised my >> patch in accordance with your first suggestion: both the old >> and the new version are present. This patch is attached below. ASh> This version looks better, but >> +ex remove_dirac_ONE(const ex & e, bool *success, unsigned char >> rl = 0, unsigned options = 0); + ASh> it still subtly changes the meaning of ASh> remove_dirac_ONE(e, 0, 0); ASh> Before the patch it was remove_dirac_ONE(e, /* rl = */ 0, /* ASh> options = */ 0), and with patch it's remove_dirac_ONE(e, /* ASh> success = */ 0, /* rl = */ 0) which ASh> 1) does not throw an exception on error 2) segfaults ASh> Please change the prototype (of the non-throwing function) to ASh> ex remove_dirac_ONE(const ex& e, bool& success, unsigned char ASh> rl = 0, unsigned options = 0); ASh> This way the compiler can pick the correct overload ASh> e = remove_dirac_ONE(e, 0, 0); // OK: remove_dirac_ONE(const ASh> ex&, unsigned char, unsigned) bool ok = false; e = ASh> remove_dirac_ONE(e, ok, 0, 0); >> Regarding crashes of my application the situation was solved by >> partial substitution: I was able to avoid crashes keeping the >> original version of GiNaC. It was enough to replace calls to >> exception-free version of remove_dirac_ONE to in my library >> only. Probably exceptions work fine within one library but >> create crashes if passed between different libraries. ASh> I don't quite understand how patching *just* remove_dirac_ONE ASh> could possibly avoid the cross-DLL exceptions problem. See, ASh> many (most?) GiNaC methods and functions use exceptions to ASh> report an error. For instance, pseries::power_const throws ASh> pole_error. Why this one is not a problem? What's special about ASh> remove_dirac_ONE? ASh> Best regards, Alexey ASh> _______________________________________________ GiNaC-devel ASh> mailing list GiNaC-devel@ginac.de ASh> https://www.cebix.net/mailman/listinfo/ginac-devel
Hi, 23.05.2019, 10:44, "Vladimir V. Kisil" <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.
Firstly making two almost identical copies of remove_dirac_ONE is definitely a bad idea. Two possible approaches are: a) wrap exception-throwing remove_dirac_ONE like this ex remove_dirac_ONE(const ex& e, bool& success, unsigned char rl, unsigned options) { ex ret; try { ret = remove_dirac_ONE(e, rl, options); } catch (...) { success = false; ret = _ex0; } return ret; } This way exceptions will propagate within ginac.dll itself only, and there's no code duplication. b) alternatively one could move the actual computation to non-throwing variant of remove_dirac_ONE, and make a wrapper which does throw exceptions: ex remove_dirac_ONE(const ex& e, unsigned char rl, unsigned options) { bool success = true; ex ret = remove_dirac_ONE(e, success, rl, options); if (!success) { // perhaps non-throwing variant should set an integer status instead of a bool flag, so a more specific exception can be thrown throw std::runtime_error("some meaningful error message here"); } return ret; } Secondly I don't quite get what's the point of replacing the throwing variant of remove_dirac_ONE, and throwing the very 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"));
I.e. why can't we keep calling ex v0 = remove_dirac_ONE(canonicalize_clifford(e+clifford_prime(e)))/2; 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.
Not really. Throwing exceptions across shared libraries (DLLs) on Windows is a little bit tricky, but it's certainly possible. The crucial point is to make all DLLs use the same C++ runtime (which implies the C++ runtime should be linked dynamically into all DLLs linked into the application). Also patching just remove_dirac_ONE is sort of pointless, since most GiNaC methods and functions throw exceptions too. Best regards, Alexey
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
Dear All, I finally managed to check Alexey's proposition to wrap the throwing remove_dirac_ONE function. It indeed prevents my Windows app from crashing. Thus I attach the respective light-weighted patches. 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
On Wed, 05 Jun 2019 20:51:10 +0100, "Vladimir V. Kisil" <kisilv@maths.leeds.ac.uk> said:
VVK> Dear All, I finally managed to check Alexey's VVK> proposition to wrap the throwing remove_dirac_ONE function. It VVK> indeed prevents my Windows app from crashing. Thus I attach the VVK> respective light-weighted patches. Apology to everyone: the previously sent version had a debugging printout. The clean patch is attached now. -- 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/ VVK> Best wishes, Vladimir -- Vladimir V. Kisil VVK> http://www.maths.leeds.ac.uk/~kisilv/ Book: Geometry of Mobius VVK> Transformations http://goo.gl/EaG2Vu Software: Geometry of VVK> cycles http://moebinv.sourceforge.net/
On Thu, 23 May 2019 15:32:12 +0400, Alexey Sheplyakov VVK> <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 VVK> From 91db78db3c8bb89a954be3f249333d3e1313b782 Mon Sep 17 VVK> 00:00:00 2001 From: "Vladimir V. Kisil" VVK> <kisilv@maths.leeds.ac.uk> Date: Tue, 14 May 2019 12:06:42 VVK> +0100 Subject: [PATCH 1/3] Suggest ios::binary flag for VVK> archiving files VVK> Without the flag locales setting of the OS tempers the file VVK> structure. VVK> Signed-off-by: Vladimir V. Kisil <kisilv@maths.leeds.ac.uk> --- VVK> doc/tutorial/ginac.texi | 8 +++++--- 1 file changed, 5 VVK> insertions(+), 3 deletions(-) VVK> diff --git a/doc/tutorial/ginac.texi b/doc/tutorial/ginac.texi VVK> index 331c2125..bc4b87fc 100644 --- a/doc/tutorial/ginac.texi VVK> +++ b/doc/tutorial/ginac.texi @@ -6743,14 +6743,16 @@ The VVK> archive can then be written to a file: VVK> @example // ... - ofstream out("foobar.gar"); + ofstream VVK> out("foobar.gar", ios::binary); out << a; out.close(); // ... VVK> @end example VVK> The file @file{foobar.gar} contains all information that is VVK> needed to -reconstruct the expressions @code{foo} and VVK> @code{bar}. +reconstruct the expressions @code{foo} and VVK> @code{bar}. The flag +@code{ios::binary} prevents locales VVK> setting of your OS tampers the +archive file structure. VVK> @cindex @command{viewgar} The tool @command{viewgar} that VVK> comes with GiNaC can be used to view @@ -6768,7 +6770,7 @@ read VVK> in again: @example // ... archive a2; - ifstream VVK> in("foobar.gar"); + ifstream in("foobar.gar", ios::binary); in VVK> >> a2; // ... @end example -- 2.20.1 VVK> From 61a028914d6e684ee50d5d32f3bdcdc7ec5f0cc0 Mon Sep 17 VVK> 00:00:00 2001 From: "Vladimir V. Kisil" VVK> <kisilv@maths.leeds.ac.uk> Date: Tue, 14 May 2019 13:11:13 VVK> +0100 Subject: [PATCH 2/3] Additional examples on number VVK> conversions. VVK> Some aspects of these methods are not obvious for newbies. VVK> Signed-off-by: Vladimir V. Kisil <kisilv@maths.leeds.ac.uk> --- VVK> doc/tutorial/ginac.texi | 9 +++++++++ 1 file changed, 9 VVK> insertions(+) VVK> diff --git a/doc/tutorial/ginac.texi b/doc/tutorial/ginac.texi VVK> index bc4b87fc..65ca5918 100644 --- a/doc/tutorial/ginac.texi VVK> +++ b/doc/tutorial/ginac.texi @@ -1533,6 +1533,15 @@ rational VVK> number will return a floating-point approximation. Both VVK> @code{to_int()/to_long()} and @code{to_double()} discard the VVK> imaginary part of complex numbers. VVK> +Note the signature of the above methods, you may need to apply VVK> a type +conversion and call @code{evalf()} as shown in the VVK> following example: +@example + ... + ex e1 = 1, e2 = VVK> sin(Pi/5); + cout << ex_to<numeric>(e1).to_int() << endl + << VVK> ex_to<numeric>(e2.evalf()).to_double() << endl; + ... +@end VVK> example VVK> @node Constants, Fundamental containers, Numbers, Basic VVK> concepts @c node-name, next, previous, up -- 2.20.1 VVK> From 284c38d43c8579ef59a7b4422150923f1d9d8e93 Mon Sep 17 VVK> 00:00:00 2001 From: "Vladimir V. Kisil" VVK> <kisilv@maths.leeds.ac.uk> Date: Tue, 14 May 2019 20:42:50 VVK> +0100 Subject: [PATCH 3/3] Modify remove_dirac_ONE() to VVK> eliminate exceptions. VVK> Previously remote_dirac_ONE() reported that an expression is VVK> not a Clifford scalar by throwing an exception. This produced VVK> crashes in Qt applications on Windows. Now exceptions are only VVK> used within the recursive calls of remote_dirac_ONE(). There is VVK> a possibility to pass the information on non-scalar expression VVK> through a Boolean variable. VVK> This patch also adds some additional test to exam_clifford.cpp VVK> and improves the tutorial description of the method. VVK> Signed-off-by: Vladimir V. Kisil <kisilv@maths.leeds.ac.uk> --- VVK> check/exam_clifford.cpp | 15 ++++++++++++--- VVK> doc/tutorial/ginac.texi | 15 +++++++++++++-- ginac/clifford.cpp VVK> | 23 +++++++++++++++++++---- ginac/clifford.h | 15 VVK> +++++++++++++-- 4 files changed, 57 insertions(+), 11 VVK> deletions(-) VVK> diff --git a/check/exam_clifford.cpp b/check/exam_clifford.cpp VVK> index b067438b..9f1361fa 100644 --- a/check/exam_clifford.cpp VVK> +++ b/check/exam_clifford.cpp @@ -417,9 +417,18 @@ template VVK> <typename IDX> unsigned clifford_check6(const matrix &A) VVK> realsymbol s("s"), t("t"), x("x"), y("y"), z("z"); VVK> ex c = clifford_unit(nu, A, 1); - e = VVK> lst_to_clifford(lst{t, x, y, z}, mu, A, 1) * VVK> lst_to_clifford(lst{1, 2, 3, 4}, c); - e1 = VVK> clifford_inverse(e); - result += VVK> check_equal_simplify_term2((e*e1).simplify_indexed(), VVK> dirac_ONE(1)); + lst elem = { dirac_ONE(1), /* Clifford scalar VVK> */ + lst_to_clifford(lst{1, 0, 0, 0}, mu, A, 1), /* Clifford VVK> vector */ + lst_to_clifford(lst{t, x, y, z}, mu, A, 1) * VVK> lst_to_clifford(lst{1, 2, 3, 4}, c) /* Clifford bi-vector */ + VVK> }; + + for (int i = 0; i < 3; ++i) { + e1 = VVK> clifford_inverse(elem[i]); + result += VVK> check_equal_simplify_term2((elem[i]*e1).simplify_indexed(), VVK> dirac_ONE(1)); + e = 3*pow(2*Pi, i)*elem[i]; + e1 = VVK> clifford_inverse(e); + result += VVK> check_equal_simplify_term2((e*e1).simplify_indexed(), VVK> dirac_ONE(1)); + } VVK> /* lst_to_clifford() and clifford_to_lst() check for vectors*/ VVK> e = lst{t, x, y, z}; diff --git a/doc/tutorial/ginac.texi VVK> b/doc/tutorial/ginac.texi index 65ca5918..c383b139 100644 --- VVK> a/doc/tutorial/ginac.texi +++ b/doc/tutorial/ginac.texi @@ VVK> -3573,12 +3573,23 @@ then an exception is raised. @cindex VVK> @code{remove_dirac_ONE()} If a Clifford number happens to be a VVK> factor of @code{dirac_ONE()} then we can convert it to a VVK> ``real'' (non-Clifford) -expression by the function +expression VVK> by the functions VVK> @example - ex remove_dirac_ONE(const ex & e); + ex VVK> remove_dirac_ONE(const ex & e, bool & success, unsigned char rl VVK> = 0); + ex remove_dirac_ONE(const ex & e, unsigned char rl = VVK> 0); @end example VVK> +The functions removes only @code{dirac_ONE()} with a VVK> representation +label not smaller than VVK> @code{rl}. Correspondingly, the default value of +@code{rl=0} VVK> removes all @code{dirac_ONE()}. If the first argument is +not VVK> a scalar multiple of @code{dirac_ONE()} and an output of the VVK> +functions is not predictable, then the first method reports VVK> this back by +the value @code{false} of the Boolean variable VVK> @code{success}. The +second method in the case of a non-scalar VVK> Clifford numbers aborts with +an exception raised. Usage of the VVK> second method may crash in Windows +threaded applications. + VVK> @cindex @code{canonicalize_clifford()} The function VVK> @code{canonicalize_clifford()} works for a generic Clifford VVK> algebra in a similar way as for Dirac gammas. diff --git VVK> a/ginac/clifford.cpp b/ginac/clifford.cpp index VVK> 860cce46..51b92599 100644 --- a/ginac/clifford.cpp +++ VVK> b/ginac/clifford.cpp @@ -1142,6 +1142,21 @@ ex VVK> clifford_prime(const ex & e) return e; } VVK> +ex remove_dirac_ONE(const ex & e, bool & success, unsigned VVK> char rl, unsigned options) +{ + ex ret; + std::cerr << "New VVK> remove_dirac";e.dbgprint(); + try { + ret = remove_dirac_ONE(e, VVK> rl, options); + success = true; + } catch (...) { + success = VVK> false; + ret = _ex0; + } + return ret; + +} + ex VVK> remove_dirac_ONE(const ex & e, unsigned char rl, unsigned VVK> options) { pointer_to_map_function_2args<unsigned char, VVK> unsigned> fcn(remove_dirac_ONE, rl, options | 1); @@ -1152,13 VVK> +1167,13 @@ ex remove_dirac_ONE(const ex & e, unsigned char rl, VVK> unsigned options) e1 = expand_dummy_sum(e, true); e1 = VVK> canonicalize_clifford(e1); } - + if (is_a<clifford>(e1) && VVK> ex_to<clifford>(e1).get_representation_label() >= rl) { if VVK> (is_a<diracone>(e1.op(0))) return 1; - else + else VVK> throw(std::invalid_argument("remove_dirac_ONE(): expression is VVK> a non-scalar Clifford number!")); - } else if (is_a<add>(e1) || VVK> is_a<ncmul>(e1) || is_a<mul>(e1) + } else if (is_a<add>(e1) || VVK> is_a<ncmul>(e1) || is_a<mul>(e1) || is_a<matrix>(e1) || VVK> e1.info(info_flags::list)) { if (options & 3) // is a child or VVK> was already expanded return e1.map(fcn); @@ -1177,7 +1192,7 @@ VVK> ex remove_dirac_ONE(const ex & e, unsigned char rl, unsigned VVK> options) } catch (std::exception &p) { need_reevaluation = VVK> true; } - } + } if (need_reevaluation) return VVK> remove_dirac_ONE(e, rl, options | 2); return e1; diff --git VVK> a/ginac/clifford.h b/ginac/clifford.h index c076d615..dfa25347 VVK> 100644 --- a/ginac/clifford.h +++ b/ginac/clifford.h @@ -294,11 VVK> +294,22 @@ inline ex clifford_bar(const ex & e) { return VVK> clifford_star_bar(e, true, 0); } inline ex clifford_star(const VVK> ex & e) { return clifford_star_bar(e, false, 0); } VVK> /** Replaces dirac_ONE's (with a representation_label no less VVK> than rl) in e with 1. - * For the default value rl = 0 remove VVK> all of them. Aborts if e contains any + * For the default value VVK> rl = 0 remove all of them. Any clifford_unit with any VVK> representation_label + * will be replaces by zero. An VVK> occurrence of a such clifford_unit can be reported through the VVK> false + * value of the success parameter. + * + * @param e VVK> Expression to be processed + * @param success It is changed to VVK> false if there is at least one clifford_unit() in the VVK> expression + * @param rl Value of representation label, all VVK> dirac_ONE with this or greater value will be processed + * VVK> @param options Defines some internal value for recursive calls, VVK> shall be ommited in user code */ +ex remove_dirac_ONE(const ex VVK> & e, bool & success, unsigned char rl = 0, unsigned options = VVK> 0); + +/** Replaces dirac_ONE's (with a representation_label no VVK> less than rl) in e with 1. + * For the default value rl = 0 VVK> remove all of them. Aborts (raise an exception) if e contains VVK> any * clifford_unit with representation_label to be removed. * VVK> * @param e Expression to be processed - * @param rl Value of VVK> representation label + * @param rl Value of representation VVK> label * @param options Defines some internal use */ ex VVK> remove_dirac_ONE(const ex & e, unsigned char rl = 0, unsigned VVK> options = 0);
Dear Alexey, Many thanks for the detailed advise. In fact I have already fixed my application by calling a local substitute of GiNaC::remove_dirac_ONE(). Since scalarisation like x*ONE - > x (or checking if it is possible at all) is a rather common task, I thought a user can benefit from non-throwing version in GiNaC. But I am not insisting by any means. Shall I write a documentation patch advising a user to make cut&paste of the GiNaC code of remove_dirac_ONE() if they will meet this type of problem? 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 Sat, 08 Jun 2019 10:25:36 +0400, Alexey Sheplyakov <asheplyakov@yandex.ru> said:
ASh> Hi, ASh> 07.06.2019, 11:45, "Vladimir V. Kisil" ASh> <kisilv@maths.leeds.ac.uk>: >>>>>>> On Wed, 05 Jun 2019 20:51:10 +0100, "Vladimir V. Kisil" ASh> <kisilv@maths.leeds.ac.uk> said: >> VVK> Dear All, I finally managed to check Alexey's proposition to VVK> wrap the throwing remove_dirac_ONE function. It indeed prevents VVK> my Windows app from crashing. Thus I attach the respective VVK> light-weighted patches. >> >> Apology to everyone: the previously sent version had a debugging >> printout. The clean patch is attached now. ASh> I think the 1st and the 2nd patches are OK and can be applied. ASh> The one which adds a non-throwing remove_dirac_ONE() is much ASh> better too, however, I don't think adding non-throwing variant ASh> of every GiNaC method and function is a good idea. Instead I ASh> suggest to fix the application itself (more details below). >> Thanks for further comment. Before making the respective change >> to the patch I wish to discuss the crash. It is indeed look >> strange (and took time to debug), but presence of exception >> within the GiNaC library was not an issue, but calling those >> methods from another library produced a crash. I do not >> understand coding well, by my guess would be that a Qt >> application run each library in a separate thread ASh> Nope, Qt applications don't (and can't) do that >> and exceptions are fine within one tread. ASh> Not only it's not right, it's not even wrong. ASh> ELF (Linux' binaries/shared libraries files format) and PE/COFF ASh> (windows' executables/shared libraries format) use completely ASh> different paradigms for run-time loading of code. ASh> ELF shared library contains code to be used by the program, and ASh> also the names of functions and data that it expects to find in ASh> other shared libraries and/or the program. When the shared ASh> library A is joined to the program, all references to those ASh> functions and data in A's code are changed to point to the ASh> actual locations in other shared libraries and the program ASh> where the functions and data are placed in memory. This is ASh> basically a usual link operation. ASh> On the other hand PE/COFF shared library (DLL) is sort of ASh> self-contained: access to external functions and data goes ASh> through a lookup table (of that DLL). So the DLL code does not ASh> have to be fixed up at runtime to refer to the program’s ASh> memory; instead, the code already uses the DLL’s lookup table, ASh> and the lookup table is modified at runtime to point to the ASh> functions and data. ASh> The key difference is that each DLL has its own lookup table, ASh> and ELF has (sort of) a single lookup table for a whole ASh> program. ASh> Thus if a program uses two DLLs, A and B, malloc/free functions ASh> referenced by DLL A can be different from malloc/free functions ASh> used by DLL B. If such a program ends up allocating memory in ASh> DLL A and releasing it in DLL B, the result is (typically) a ASh> crash. ASh> A similar problem exists with C++ exceptions. The internal data ASh> used to find the matching catch block and perform stack ASh> unwinding are highly compiler-specific and may use global ASh> variables and heap. Thus if all DLLs aren't sharing C++ runtime ASh> cross-DLL exception (throwing an exception in DLL A and ASh> catching it in DLL B) is a problem (most likely crash). ASh> To recap: on windows exceptions are fine ASh> - within the same DLL - across DLLs sharing the same (C/C++) ASh> runtime ASh> Hope this helps, Alexey ASh> _______________________________________________ GiNaC-devel ASh> mailing list GiNaC-devel@ginac.de ASh> https://www.cebix.net/mailman/listinfo/ginac-devel
participants (2)
-
Alexey Sheplyakov
-
Vladimir V. Kisil