-
Notifications
You must be signed in to change notification settings - Fork 841
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
Deprecate outflow generalized #390
Conversation
…e future. Functionality preserved in a separate branch
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.
Looking good! Just a couple of comments.
@@ -3490,51 +3489,11 @@ void CAdjIncEulerSolver::BC_Outlet(CGeometry *geometry, CSolver **solver_contain | |||
case MASS_FLOW_RATE: | |||
Psi_outlet[0]+=1; | |||
break; | |||
case OUTFLOW_GENERALIZED: |
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.
I noticed this is in the incompressible adjoint solver. did these outlet options work within that solver too? Perhaps we need to trim down more?
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.
I will remove this shortly. I think these were copied over when someone created that *_inc file - these boundary conditions would NOT be expected to work with the incompressible solver; there is literature on incompressible adjoints for a few objectives, and the equations will be different.
@@ -196,16 +196,14 @@ MG_ADJFLOW= NO | |||
% INVERSE_DESIGN_HEATFLUX, AVG_TOTAL_PRESSURE, | |||
% MASS_FLOW_RATE) | |||
% For a weighted sum of objectives: separate by commas, add OBJECTIVE_WEIGHT and MARKER_MONITORING in matching order. | |||
OBJECTIVE_FUNCTION= OUTFLOW_GENERALIZED | |||
OBJECTIVE_FUNCTION= AVG_TOTAL_PRESSURE |
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.
Great - thanks for updating the test to check the total pressure / 1D stuff.
@@ -135,7 +135,7 @@ def main(): | |||
# test continuous_adjoint.py, with multiple objectives | |||
discadj_multi_py = TestCase('discadj_multi_py') | |||
discadj_multi_py.cfg_dir = "cont_adj_euler/wedge" | |||
discadj_multi_py.cfg_file = "inv_wedge_ROE_disc_multiobj.cfg" | |||
discadj_multi_py.cfg_file = "inv_wedge_ROE_multiobj.cfg" |
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.
Just to be clear: the same config file now works for both continuous and discrete, correct?
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.
Yes.
…s that references test cases branch (revert to develop version of .travis before merge)
…code/SU2 into deprecate_OutflowGeneralized
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.
LGTM!
This pull request deprecates the generalized outflow adjoint. This functionality is still available in an alternate branch, however I won't be able to maintain it for future updates.
This means that some of the regression tests had to be updated, since they previously included testing this functionality.
This pull request uses a matching branch in the TestCases repository to ease that update - the .travis.yml file will can be switched back to the TestCases/develop branch when needed.