-
Notifications
You must be signed in to change notification settings - Fork 918
Cleanup python wrapper, replace legacy python FSI #1197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| if ((config.GetnMarker_Fluid_Load() > 0) && (MGLevel == MESH_0)) { | ||
| Alloc3D(nMarker, nVertex, nDim, VertexTraction); | ||
| if (config.GetDiscrete_Adjoint()) Alloc3D(nMarker, nVertex, nDim, VertexTractionAdjoint); | ||
| if (MGLevel == MESH_0) { | ||
| VertexTraction.resize(nMarker); | ||
| for (iMarker = 0; iMarker < nMarker; iMarker++) { | ||
| if (config.GetSolid_Wall(iMarker)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is to allocate and compute for all solid walls since this is not very expensive.
| PyWrapNodalForce[iDim] = -(Pn-Pinf)*Normal[iDim]; //NB : norm(Normal) = Area | ||
| } | ||
| /*--- Get the values of pressure and viscosity ---*/ | ||
| Pn = solver_container[ZONE_0][INST_0][MESH_0][FLOW_SOL]->GetNodes()->GetPressure(iPoint); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit different from the function compute_vertex_traction in Csolver.cpp. For example, the Pinf there is equal to config->GetPressure_FreeStreamND and everything is later multiplied for a factor accounting for adimensionalisation. Should this part be modified to be the same?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In any case, if we now compute all the tractions inside SU2 itself, it there still the need for this function?
I think it is only used in an old test case...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it is not too much work adapt the old case we can remove this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The old test case uses a FSIdriver that I cannot find anymore, not sure if it would work in any case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad, the test case is indeed used, not with this FSIdriver, but with the singlezone. It is a forced response.
I will try update the test case to the new routines
|
|
||
| iPoint = geometry_container[ZONE_0][INST_0][MESH_0]->vertex[iMarker][iVertex]->GetNode(); | ||
| Coord = geometry_container[ZONE_0][INST_0][MESH_0]->nodes->GetCoord(iPoint); | ||
| auto iPoint = geometry_container[ZONE_0][INST_0][MESH_0]->vertex[iMarker][iVertex]->GetNode(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These functions also were all intended to be used with the old mesh solver. Now that we do not provide anymore the change in coordinates to the geometry container but to the mesh solver they should not be used anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it does not work it should be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okok, I will do that
| vector<su2double> NodalForce (3,0.0); | ||
| NodalForce[0] = LoadX; | ||
| NodalForce[1] = LoadY; | ||
| NodalForce[2] = LoadZ; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pattern in these functions is to load the 3 separate components into an array or vector to then pass by pointer to the solver.
Since we know the number of components (from the function signature) it is better to use a fixed-size temporary for that, e.g. su2double NodalForce[3]; or array<su2double,3> NodalForce, since that avoids dynamic allocation when this function is called for each vertex.
For the functions that return small vectors we would ideally do the same, return a fixed size type (the only choice there would be array<su2double,3>) but maybe python/swig has some limitation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much for the comment!
I do not think swig is limited in that sense, the limitation was mine not perfectly understanding the difference, I see now.
I am changing it
…leanup_python_wrapper
…leanup_python_wrapper
…leanup_python_wrapper
| PyWrapNodalHeatFlux[0] = 0.0; | ||
| PyWrapNodalHeatFlux[1] = 0.0; | ||
| PyWrapNodalHeatFlux[2] = 0.0; | ||
|
|
||
| } | ||
|
|
||
| passivedouble CDriver::Get_Drag() { | ||
| ///////////////////////////////////////////////////////////////////////////// | ||
| /* Functions related to the global performance indices (Lift, Drag, ecc..) */ | ||
| ///////////////////////////////////////////////////////////////////////////// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to see those temporaries going away and the different methods organized 👍
Proposed Changes
Trying to make the python vertex tractions and vertex traction adjoints independent of a specific marker to allow #1174 to go forward without side effects on how FSI is done via the python wrapper at the moment.
Related Work
#1174
PR Checklist