[GiNaC-devel] remove_dirac_ONE() and documentation patches

[Vladimir V. Kisil]

	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 at YANDEX.ru> said:

    ASh> Hi,

    ASh> 23.05.2019, 10:44, "Vladimir V. Kisil"
    ASh> <kisilv at 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