FluidityProject / fluidity

Fluidity
http://fluidity-project.org
Other
365 stars 115 forks source link

Improved error messages from python #321

Closed stephankramer closed 3 years ago

stephankramer commented 3 years ago

Often when an error occurs in the python embedded in fluidity, the error+backtrace would not show up despite use calling PyErr_Print and only prints the useless FLAbort message "Dying!". This is caused by some internal buffering in python of stdio and stderr (internal to python so independent of whether we have redirected the actual stdio/stderr of the process at the system level). This commit switches all such buffering off. As suggested here: https://stackoverflow.com/questions/29352485/python-print-not-working-when-embedded-into-mpi-program/49076389#49076389 under option 1) Note that it should only affect stdio/err and not all I/O from python (as it claims on stackexchange), and since we shouldn't normally be printing much from python this should hopefully not affect performance.

Also fixed a case where the error would result in a segfault because we try to decref the pyobject returned from PyRun_String before checking whether an error has occured, in which case PyRun_String returns null.

This came up in #315

jhill1 commented 3 years ago

Nice. I don't see any major issues with this and well found on the fix!

stephankramer commented 3 years ago

Ah great, thanks for digging that up. Right, so if I understand correctly there's basically three different modes 1) block buffering (which is the normal default for all files) 2) line buffering 3) immediate write through. I think Py_UnbufferedStdioFlag was intended (in python 2) to give you write through, but according to the discussion that wasn't available in early python3. So that's why prior to 3.7 it just switched to line buffering instead (instead of block buffering). Then in 3.7 they realized the functionality was now available so you do get write through with that flag now. I don't think anything changes in 3.9 if you have that flag set, I presume it means unbuffered_studio will be false. It's just that in 3.9 without that flag it will always use line buffering (instead of block) for stderr - even if it's not a tty. The issue we were having is that in parallel stderr fails the isatty test even if we don't redirect ourselves (presumably because it's redirected by mpi before being dumped in the terminal). For our purpose I guess line buffering should be sufficient which is what we get by default in 3.9 - but for older versions there seems to be no simple way to achieve that.