Fix inverse design Cp function #1311
Conversation
| } | ||
| } | ||
| } | ||
| if (!Surface_file.good()) { |
There was a problem hiding this comment.
Why not erroring in case the file is no bueno? "Unable to read from SurfaceCP file"
Currently it is some sort of silent error catch that gives "false" results but does not really communicate why. Or am I overlooking sth here
There was a problem hiding this comment.
Ah! Because this function is always called and the result is always available as one of the outputs 🤷 and so it cannot throw an error.
There was a problem hiding this comment.
(I missed the last bus to this PR 🚏 🚌 ... so I am arriving just now 🐌 )
Thanks for getting into the details of this implementation and fixing this @pcarruscag 💐
| * \param[in] geometry - Geometrical definition of the problem. | ||
| * \param[in] config - Definition of the particular problem. | ||
| */ | ||
| void Set_CpInverseDesign(CSolver *solver, CGeometry *geometry, CConfig *config); | ||
| void Set_CpInverseDesign(CSolver *solver, const CGeometry *geometry, const CConfig *config); |
There was a problem hiding this comment.
👍 for making the interface clearer with the consts
| @@ -69,9 +69,9 @@ def main(): | |||
| discadj_arina2k.cfg_dir = "disc_adj_euler/arina2k" | |||
| discadj_arina2k.cfg_file = "Arina2KRS.cfg" | |||
| discadj_arina2k.test_iter = 20 | |||
| discadj_arina2k.test_vals = [2.189902, 1.635938, 47258.000000, 0.000000] | |||
| discadj_arina2k.test_vals = [-3.111181, -3.501516, 6.8705e-02, 0] | |||
There was a problem hiding this comment.
I guess the first to fields are residuals (i.e. quite an improvment by this bugfix), and good to see this fix is directly captured by the reg-test ... but this also tells 'dont trust the regression tests' :)
There was a problem hiding this comment.
garbage in garbage out as with all things
| } | ||
| } | ||
| } | ||
| if (!Surface_file.good()) { |
There was a problem hiding this comment.
Why not erroring in case the file is no bueno? "Unable to read from SurfaceCP file"
Currently it is some sort of silent error catch that gives "false" results but does not really communicate why. Or am I overlooking sth here
| if (!set) | ||
| cout << "WARNING: In file " << surfCp_filename << ", point " << iPointGlobal << " is not a vertex." << endl; |
There was a problem hiding this comment.
👍 for this warning (and some additional comments in this section)
| const auto Cp = solver->GetCPressure(iMarker, iVertex); | ||
| const auto CpTarget = solver->GetCPressureTarget(iMarker, iVertex); | ||
|
|
||
| PressDiff += Area * (CpTarget - Cp) * (CpTarget - Cp); | ||
| } | ||
| const auto Normal = geometry->vertex[iMarker][iVertex]->GetNormal(); | ||
| const auto Area = GeometryToolbox::Norm(nDim, Normal); |
There was a problem hiding this comment.
Just a note: We need to be careful on future PR's with these su2double auto's because like so as a single assignment it is correct but is looks straight forward e.g. to extend it to const auto Cp_rho = solver->GetCPressure(iMarker, iVertex) * rho (rho falling from the sky) which would then be incorrect because the auto for su2double active then evaluates to an expression.
| const auto iPoint = geometry->vertex[iMarker][iVertex]->GetNode(); | ||
| if (!geometry->nodes->GetDomain(iPoint)) continue; |
There was a problem hiding this comment.
Fai (For anyone's interest): this is the missing check of domain points that is mentioned in the cfd-online post
Proposed Changes
It had mpi problems and others: https://www.cfd-online.com/Forums/su2/236677-mpi-returns-wrong-objective-function-values.html
The target Cp file was also not formatted correctly, the residuals will change because of that.
PR Checklist