Handle potential exceptions from preCICE calls #366
Conversation
|
The system tests pass, the issues previously were related to the runner: https://github.com/precice/openfoam-adapter/actions/runs/14330310572/job/40172178566?pr=366 |
There was a problem hiding this comment.
Not sure if there is any point in actually including the .what(). This should always be duplicated, right? Including it leads to this:
Actually no, I don't think so. It should only appear if we actually print (using what) in the catch block. It could be related to how we interface with boost log, I vaguely remember some issues we had with ASTE as well.
Regarding this issue. I am not 100% convinced of all the try's which are now sprinkled across the entire code. A less scattered alternative would be to wrap highlevel functions in the Adapter class in a try-catch block.
We could still catch the precice exceptions using using catch(::precice::Error). This would not work with former preCICE versions then, but we can use our version macros to distinguish and have something like
#if PRECICE_VERSION_GTE(3,2,0)
using PreciceError = ::precice::Error
#else
using PreciceError = std::runtime_error
#endifIf the else branch becomes unnecessary at some point, we can remove it and have the clean solution.
| bool requiresInitialData = false; | ||
| try | ||
| { | ||
| precice_->requiresInitialData(); | ||
| } | ||
| catch (const std::runtime_error& e) | ||
| { | ||
| std::exit(1); | ||
| } | ||
| if (requiresInitialData) | ||
| { | ||
| DEBUG(adapterInfo("Initializing preCICE data...")); | ||
| writeCouplingData(); | ||
| } | ||
|
|
||
| DEBUG(adapterInfo("Initializing preCICE data...")); | ||
| precice_->initialize(); | ||
| try | ||
| { | ||
| precice_->initialize(); | ||
| } | ||
| catch (const std::runtime_error& e) |
There was a problem hiding this comment.
That's a rather verbose way of handling this, also a bit hard to read. Why not simply wrapping the whole code block into a try catch?
try{
if (precice_->requiresInitialData())
precice_->initialize()
} catch(...)
{}
}| } | ||
| catch (const std::runtime_error& e) | ||
| { | ||
| std::exit(1); |
There was a problem hiding this comment.
| std::exit(1); | |
| std::exit(EXIT_FAILURE); |
| @@ -702,6 +734,20 @@ void preciceAdapter::Adapter::adjustSolverTimeStepAndReadData() | |||
| return; | |||
| } | |||
|
|
|||
| double preciceAdapter::Adapter::getMaxTimeStepSize() | |||
There was a problem hiding this comment.
| double preciceAdapter::Adapter::getMaxTimeStepSize() | |
| double preciceAdapter::Adapter::getMaxTimeStepSize() const |
?
Closes #362.
Every exception is just handled by throwing an error message, but this still leads to nicer output than the complete call stack. For example, simply:
compared to previous state:
Not sure if there is any point in actually including the
.what(). This should always be duplicated, right? Including it leads to this:Also not sure if we want to actually trigger a
Foam::FatalErrorvia our logger. It does not really add much more information, besides that the error comes from the adapter.@fsimonis can we assume that some API methods (such as the boolean checks) never throw? Would a
noexceptkeyword in these API methods make sense?TODO list:
docs/-> N/Achangelog-entries/(create directory if missing)