Simplify error handling (remove error-deferred workaround) #364
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The system tests now pass: https://github.com/precice/openfoam-adapter/actions/runs/14307651426/job/40095066009?pr=364 Now only waiting for review. |
davidscn
approved these changes
May 5, 2025
| @@ -365,6 +366,8 @@ This directs the solver to use the `preciceAdapterFunctionObject` function objec | |||
| which is part of the `libpreciceAdapterFunctionObject.so` shared library. | |||
| The name `preCICE_Adapter` can be arbitrary. It is important that the library is loaded outside the `functions` dictionary when you want to use the custom boundary conditions that we provide with the FF module. | |||
|
|
|||
| The `errors strict` option is optional and [available since OpenFOAM v2012](https://www.openfoam.com/news/main-news/openfoam-v20-12/post-processing#post-processing-function-object-error-handling). Since the adapter is necessary to do a coupled simulation, this option instructs OpenFOAM to stop in case it faces issues with loading the adapter. For OpenFOAM versions that don't support this, remove the option. | |||
There was a problem hiding this comment.
So, if not set, my OpenFOAM solver will just keep going, yes?
There was a problem hiding this comment.
Correct, same as when OpenFOAM cannot find the adapter.
Do you think this is particularly risky?
Should we take any further actions beyond mentioning this prominently in the changelog?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, we have a workaround to force OpenFOAM to stop when encountering an error in the configuration phase. This is not needed anymore since OpenFOAM v2012, which introduces an additional option
errorsfor the function object, which if set tostrict, acts as we would expect in this scenario. See related OpenFOAM issue.The
errorsoption is now in OpenFOAM long enough that we can assume we will not trouble too many users.This PR:
error-deferredoption fromUtilities.Cerror-deferredwitherrorFataErrorInFunctioninUtilities.Cwith aFatalError(theFatalErrorInFunctiondid not seem to work in some cases, and it does not seem appropriate for a central logging function)FatalErrorInFunctionwith calls to theadapterInfofunction, clarifying the respective error messages, if needed.Ideally, I would go as far as to rename
adapterInfotoadapterLog, but this would touch too many files and potentially frustrate some users that maintain their own forks.I have tested the following:
patchNamereadDatarho-> a default OpenFOAM error is raised during the lookupReviewers: Filter out the whitespace changes. I indented-out a large part of
Adapter.Cbecause I removed a try-catch construct.TODO list:
docs/changelog-entries/(create directory if missing)