improve returned data if running a simulation (using the model executable)#398
improve returned data if running a simulation (using the model executable)#398syntron wants to merge 3 commits intoOpenModelica:masterfrom
Conversation
| cwd=cmd_run_data.cmd_cwd_local, | ||
| timeout=cmd_run_data.cmd_timeout, | ||
| check=True, | ||
| check=False, |
There was a problem hiding this comment.
Getting an exception on non-zero exit code was good. Exceptions for handling control flow in Python are totally fine, and obvious to the user, and hard to miss, I would keep that. (I did not realize that that's what's happening because I did not dig deep enough in the call hierarchy)
The additional logging is good, so I'd keep that too. 👍
There was a problem hiding this comment.
The exception on non-zero exit code has to be handled with care. There are possibilities that a result file is created (and usefull) even if the exit code is != 0. An exception here would prevent any processing of such data.
If I remeber correctly, an example is if the model is terminated via Modelica code.
There was a problem hiding this comment.
If I remeber correctly, an example is if the model is terminated via Modelica code.
You mean via terminate()? If yes a nonzero exit code would be strange, because this "successfully terminates the analysis which was carried out".
There was a problem hiding this comment.
For me, this change is fine. It’s also consistent with how we handle this in OMEdit: we show the user what happened when running the executable, and if there’s any useful output, we use it for post-processing. If an exception is raised, that post-processing will not happen anyway.
Regarding check=True, it does the following:
If check is True and the exit code is non-zero, a CalledProcessError is raised. The CalledProcessError object contains the return code in the returncode attribute, and the output and stderr attributes if those streams were captured.
The new code already covers this behavior: it explicitly checks the returncode and logs it.
…table * use check=False => no CalledProcessError exception; possibility to handle the error code * error code needs to be checked (compare == 0) by the caller! * improve log messages; print returncode if it is != 0 with stdout
* ensure a message if logged if returncode != 0
* self.session().run_model_executable() will raise OMCSessionException!
see #108
[OMCSession] improve log messages for model simulation using OM executable
[ModelicaSystem] improve handling of model simulation