Error handling in sparselizard and spylizard

[Ehtycs]

Hi,

I started to play around with spylizard and so far it looks great. There is however one major flaw, which is the error handling.

Any error is handled using "abort" in the c++ world, which makes the whole program crash. In spylizard it has the side-effect that it crashes the python interpreter and ipython kernel too. In Jupyter notebooks the error message doesn't get printed anywhere, the program just stops with "the kernel seems to have died".

For example, if I have a line: magdyn += integral(magnetic, -grad(dof(A)) * nu * grad(tf(A))) where the error is that grad's should be curls it prints this error message Error in 'expression' object: trying to multiply a 3x2 expression by a 3x2 and crashes the kernel, losing all context and not even telling the line where the error was hit. The code needs to be re-run line by line to catch the error location manually.

The optimal case for spylizard would be for it to raise an exception with that given error message. This would not crash the kernel but instead lets the user see where the error is, fix it and continue executing.

So as a suggestion, I think this requires replacing the c++ "abort" calls with something more graceful, like an exception. Seems that pybind is capable of translating c++ exceptions to python exceptions automatically, so it wouldn't be too much work.

If you work only in c++ with sparselizard, uncaught exceptions still create a core dump and crash the program so the behaviour doesn't change.

[halbux]

Good point. Already raised by @hbadi . Problem is: there is no guarantee that the object state has not been invalidated after the abort, so it is a dangerous thing to let the user continue where it failed. This could be doable anyways but if would require carefully checking that the object state is not altered at every error call.

[Ehtycs]

Oh, yes. If there are any side-effects which happen before the error, then it goes wrong. But still, this has clear benefits. You can document the behaviour and instruct users not to do that :)

I conjured this search: \n(\s*)std::cout(.+);\n\s*abort\(\); and replace \n$1std::stringstream tmp;\n$1tmp $2;\n$1throw std::runtime_error(tmp.str()); regex and ran it through the codebase (~600 locations) replacing the error messages and aborts with std::runtime_error.

Now at least in this case the error works as I proposed above, of course the object state invalidation still remains.

[halbux]

Thanks for the automatic way of replacing all! Will do asap

[Ehtycs]

No problem. I could also have done a PR if you like. Let me know.