Skip to content
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

Custom objective function (e.g. 'DRAG + LIFT') #1478

Merged
merged 13 commits into from
Jan 9, 2022

Conversation

pcarruscag
Copy link
Member

Proposed Changes

Allows specifying an objective function via a math expression using the history outputs and typical math functions.
General cleanup of dead code and output fixes.

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.

@lgtm-com
Copy link

lgtm-com bot commented Jan 1, 2022

This pull request fixes 2 alerts when merging 4d51419 into 9efe761 - view on LGTM.com

fixed alerts:

  • 2 for Comparison of narrow type with wide type in loop condition

@lgtm-com
Copy link

lgtm-com bot commented Jan 1, 2022

This pull request fixes 2 alerts when merging d16a78e into 9efe761 - view on LGTM.com

fixed alerts:

  • 2 for Comparison of narrow type with wide type in loop condition

@TobiKattmann
Copy link
Contributor

Me: eats gingerbread for 3 weeks
Pedro: Casually writes and adds a Math Expression parser and connects that to the OF's
FeelsBad.man

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 @pcarruscag 💐 I've been looking at the CFX Expression Language with envy for quite some time now. And this is a good push into that direction (although not quite as mighty as the CFX CEL yet ;) ). The possibility to specify a per-surface output is really the 🍒 on the 🍰 (i.e. The cherries on the strawberry cake 👍 )

Good job

string("For multiple surfaces per objective, either use one objective or list the objective multiple times.\n") +
string("For multiple objectives per marker either use one marker or list the marker multiple times.\n")+
string("Similar rules apply for multi-objective optimization using OPT_OBJECTIVE rather than OBJECTIVE_FUNCTION."),
SU2_MPI::Error("When using more than one OBJECTIVE_FUNCTION, MARKER_MONITORING must be the same length or length 1.\n"
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 string( wrapper can be removed in quite a few places in the CConfig.cpp

Comment on lines +21 to +23
[submodule "externals/mel"]
path = externals/mel
url = https://github.com/pcarruscag/MEL.git
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, nice to have this modularized away (also with a compatible license 😉 )

Comment on lines -1566 to -1569
TOTAL_PRESSURE_LOSS = 39,
KINETIC_ENERGY_LOSS = 40,
TOTAL_EFFICIENCY = 41,
TOTAL_STATIC_EFFICIENCY = 42,
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 these are never implemented?

Copy link
Member Author

Choose a reason for hiding this comment

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

Or they were dropped in v7

Comment on lines +75 to +77
if (config->GetKind_Species_Model() != SPECIES_MODEL::NONE) {
/// DESCRIPTION: Average Species
for (unsigned short iVar = 0; iVar < config->GetnSpecies(); iVar++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

TestCases/user_defined_functions/lam_flatplate.cfg Outdated Show resolved Hide resolved
Comment on lines +19 to +28
% Here we define how the custom objective is computed from other outputs. For
% example, force in the z direction (computed for all MARKER_MONITORING and part
% of AERO_COEFF) plus the absolute value of massflow across the second surface
% ([1]) in MARKER_ANALYZE, scaled by a factor of 1000. It is also possible to
% use "per surface" values from MARKER_MONITORING (use the dry-run mode to see
% the names of available outputs, e.g. SU2_CFD -d lam_flatplate.cfg).
% For multizone problems the CUSTOM_OBJFUNC should be defined for each zone
% individually (with the outputs of that zone), the total for the problem is
% the sum over zones, see disc_adj_fsi/Airfoil_2d.
CUSTOM_OBJFUNC= '1e3 * (FORCE_Z + fabs(SURFACE_MASSFLOW[1]))'
Copy link
Contributor

Choose a reason for hiding this comment

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

🏅 Absolutely amazing! Being able to specify boundary markers is great... of course bonus points would be available for being able to specify the marker-tag instead of a number.

Copy link
Member Author

Choose a reason for hiding this comment

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

That is a bit tricky, having first to deduce if it should come from marker analyse or marker monitoring.
But the eventual user defined functions with mass flow averages and so on would use lists of marker names.

Co-authored-by: TobiKattmann <31306376+TobiKattmann@users.noreply.github.com>
@lgtm-com
Copy link

lgtm-com bot commented Jan 5, 2022

This pull request fixes 2 alerts when merging 14dabda into cdf780a - view on LGTM.com

fixed alerts:

  • 2 for Comparison of narrow type with wide type in loop condition

@lgtm-com
Copy link

lgtm-com bot commented Jan 7, 2022

This pull request fixes 2 alerts when merging 130e2dd into cdf780a - view on LGTM.com

fixed alerts:

  • 2 for Comparison of narrow type with wide type in loop condition

@lgtm-com
Copy link

lgtm-com bot commented Jan 9, 2022

This pull request fixes 2 alerts when merging 1ea8b02 into 85b2b12 - view on LGTM.com

fixed alerts:

  • 2 for Comparison of narrow type with wide type in loop condition

@pcarruscag pcarruscag merged commit f23b907 into develop Jan 9, 2022
@pcarruscag pcarruscag deleted the custom_objective_function branch January 9, 2022 16:07
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.

2 participants