You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The python wrapper instantiates drivers directly. To ensure that SU2 is initialized properly, also if used from python, some initialization code is already placed in the driver constructor. This PR moves additional initialization code to the driver constructor.
I have a couple of question and suggestions about things I noticed during moving the initialization.
SU2_DOT did not call omp_initialize(), it does now after moving omp_initialize() to the driver constructor. Does it actually run any hybrid AD stuff?
The number of OpenMP threads can be specified for SU2_CFD via the -t parameter, but for SU2_DOT and the python wrapper I saw no way to configure it. Making the number of threads a parameter of the driver constructors and calling omp_set_num_threads in the constructor of CDriverBase seems like a natural way to enable this for all flavours of SU2. Would that be a nice addition?
I couldn't find any python wrapper tests among the parallel AD and hybrid AD tests. Could any of the serial AD python wrapper tests serve as a basis for a parallel AD or hybrid AD one?
pcarruscag
changed the title
[WIP] Fix SU2 initialization when using the python wrapper
Fix SU2 initialization when using the python wrapper
Mar 15, 2023
Thanks for your approval @pcarruscag. Could I have your opinion on the questions? E.g., I could add 2. as part of this PR, now that I already looked into it.
Ah sorry,
SU2_DOT mostly only uses OpenMP in the passive linear system, I'm not sure if @thomasdick Sobolev smoother has some recorded part that involves OpenMP.
For the python wrapper I usually set the number of threads with env var OMP_NUM_THREADS It may be a bit confusing for most users having to specify the number of threads when instantiating the driver 🤔
I think disc_adj_flow would be the easiest to adapt. Those tests print a sensitivity at a particular vertex of a marker to be checked by the regression script, so you would have to make sure that's only printed on one rank (for parallel AD).
disc_adj_fea should also be fine for hybrid AD.
Thanks for your feedback! I agree, OMP_NUM_THREADS is better than having the number of threads in the constructor. I adapted disc_adj_flow and disc_adj_fea for MPI and added both to parallel AD and hybrid AD tests.
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
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.
Proposed Changes
The python wrapper instantiates drivers directly. To ensure that SU2 is initialized properly, also if used from python, some initialization code is already placed in the driver constructor. This PR moves additional initialization code to the driver constructor.
Related Work
This came up during #1903.
PR Checklist