Skip to content

Conversation

@pcarruscag
Copy link
Member

Proposed Changes

I tried to do something about the OF's that are not working with the python scripts.
If I got it right, the OF string needs to match the name used for history/screen output, at least to allow the automatically generated history map to be used.

Related Work

#1075

PR Checklist

  • I am submitting my contribution to the develop branch.
  • My contribution generates no new compiler warnings (try with the '-Wall -Wextra -Wno-unused-parameter -Wno-empty-body' compiler flags, or simply --warnlevel=2 when using meson).
  • My contribution is commented and consistent with SU2 style.
  • I have added a test case that demonstrates my contribution, if necessary.
  • I have updated appropriate documentation (Tutorials, Docs Page, config_template.cpp) , if necessary.

Comment on lines -1597 to -1598
/* DESCRIPTION: Compute buffet sensor */
addBoolOption("BUFFET_MONITORING", Buffet_Monitoring, false);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was cheap enough to compute by default.

Comment on lines -247 to -248
/// DESCRIPTION: Temperature
AddHistoryOutput("TEMPERATURE", "Temp", ScreenOutputFormat::SCIENTIFIC, "HEAT", "Total avg. temperature on all surfaces set with MARKER_MONITORING.", HistoryFieldType::COEFFICIENT);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This "temperature" output was removed from everywhere except the heat and incompressible solvers, because it is a right mess. i.e. not consistent for the different solvers because for ones it depends on marker_analyse for others on marker_monitoring... Also the name TOTAL_AVG_TEMPERATURE could as well have been ?_?_TEMPERATURE, just as descriptive.

Comment on lines -397 to -401
void CDiscAdjSolver::RegisterObj_Func(CConfig *config) {

/*--- Here we can add new (scalar) objective functions ---*/
if (config->GetnObj()==1) {
switch (config->GetKind_ObjFunc()) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not seem to do anything anymore.

@pcarruscag pcarruscag merged commit e4b1579 into develop Jan 20, 2021
@pcarruscag pcarruscag deleted the fix_objective_functions branch January 20, 2021 15:57
Copy link
Contributor

@TobiKattmann TobiKattmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for this cleanup 💐
From now on I guess the history map should never be touched by hand and only via updateHistoryMap, am I right? If yes, would it be a thing to have historyMap.py removed from the repo and automatically call updateHistoryMap.py during installation process (i.e. historyMap exists only in the build folder)?
(And of course sorry for the late comments)

}
}
} else {
} else if (false) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is the section you refer to in #1171

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep

*/
class CAvgGrad_NEMO : public CNEMONumerics {
private:
unsigned short iDim, iVar; /*!< \brief Iterators in dimension an variable. */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you get those as compiler warnings for unused variables? If yes, did you just use gcc with -Wall -Wextra or did you use any specific tool?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using clang with --warnlevel=2

@pcarruscag
Copy link
Member Author

It could be a thing to run it on installation, but at the same time it is also not a big deal to run it ourselves.
The script is not particularly robust IMO because it does not check for conflicting options coming from different solvers, e.g. same name output but a slightly different header on the history file, or a different history group, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants