-
Notifications
You must be signed in to change notification settings - Fork 905
[WIP] Multizone adjoints for turbomachinery #2446
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
base: develop
Are you sure you want to change the base?
Conversation
| AD::EndPreacc(); | ||
|
|
||
| /*--- Max is not differentiable, so we not register them for preacc. ---*/ |
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.
What is the difference between ending the preaccumulation before or after?
| } | ||
| } | ||
| } else if (Multizone_Problem && DiscreteAdjoint) { | ||
| SU2_MPI::Error(string("OUTPUT_WRT_FREQ cannot be specified for this solver " |
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.
| SU2_MPI::Error(string("OUTPUT_WRT_FREQ cannot be specified for this solver " | |
| SU2_MPI::Error("OUTPUT_WRT_FREQ cannot be specified for this solver " |
| "(writing of restart and sensitivity files not possible for multizone discrete adjoint during runtime yet).\n" | ||
| "Please remove this option from the config file, output files will be written when solver finalizes.\n"), CURRENT_FUNCTION); |
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.
Hmmm I'm pretty sure it is possible
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 one for @oleburghardt
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 and no, writing the files is fine, but the continuation of the multi-zone discrete adjoint solver is erratic. My guess is that some indices aren't cleared properly before re-recording the tape. (Writing adjoint solution variables to file only, without re-recording at all and without sensitivities, might give us some hints.)
The debug mode could eventually pin down where exactly things go wrong.
| BPressure = config->GetPressureOut_BC(); | ||
| Temperature = config->GetTotalTemperatureIn_BC(); | ||
|
|
||
| if (!reset){ | ||
| AD::RegisterInput(BPressure); | ||
| AD::RegisterInput(Temperature); | ||
| } | ||
|
|
||
| BPressure = config->GetPressureOut_BC(); | ||
| Temperature = config->GetTotalTemperatureIn_BC(); | ||
|
|
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.
Why are these wrong but not the others that follow the same pattern?
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 threw a tagging error, and if I remember correctly it's because in the other sections the variables it accesses are directly used in the solver, whereas in the Giles implementation they are not, creating a mismatch in tags.
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.
Can we fix it in a way that keeps a clear pattern for doing this type of thing?
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 can give it a go
…ding of objective function calculation
… to run in quasi-MZ approach. (NEEDS CLEANING)
… MZ turbo testcase
SU2_CFD/src/drivers/CDriver.cpp
Outdated
| } | ||
| } | ||
|
|
||
| //TurbomachineryStagePerformance = std::make_shared<CTurbomachineryStagePerformance>(solver[ZONE_0][INST_0][MESH_0][FLOW_SOL]->GetFluidModel()); |
Check notice
Code scanning / CodeQL
Commented-out code
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 days ago
The best way to fix this problem is to remove the commented-out code entirely, since it serves no purpose for reading or maintenance of the code and may lead to confusion in the future. This entails deleting line 2750 from SU2_CFD/src/drivers/CDriver.cpp. No other changes are required to code logic or structure, because this is strictly the removal of unreachable/commented code that is not being used.
| @@ -2747,7 +2747,6 @@ | ||
| } | ||
| } | ||
|
|
||
| //TurbomachineryStagePerformance = std::make_shared<CTurbomachineryStagePerformance>(solver[ZONE_0][INST_0][MESH_0][FLOW_SOL]->GetFluidModel()); | ||
| } | ||
|
|
||
| CDriver::~CDriver() = default; |
| void CDriver::PreprocessTurbomachinery(CConfig** config, CGeometry**** geometry, CSolver***** solver, | ||
| CInterface*** interface, CIteration*** iteration, bool dummy){ | ||
| unsigned short donorZone,targetZone, nMarkerInt, iMarkerInt; | ||
| unsigned short nSpanMax = 0; |
Check notice
Code scanning / CodeQL
Unused local variable
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 days ago
The best way to fix the problem is to remove the declaration of the unused local variable nSpanMax from function CDriver::PreprocessTurbomachinery in SU2_CFD/src/drivers/CDriver.cpp at line 2658. Only delete the specific line that declares nSpanMax; do not change other code or declarations unless they also reference nSpanMax (none appear to do so). This removal increases readability and avoids confusion about potential usage.
| @@ -2655,7 +2655,6 @@ | ||
| void CDriver::PreprocessTurbomachinery(CConfig** config, CGeometry**** geometry, CSolver***** solver, | ||
| CInterface*** interface, CIteration*** iteration, bool dummy){ | ||
| unsigned short donorZone,targetZone, nMarkerInt, iMarkerInt; | ||
| unsigned short nSpanMax = 0; | ||
| bool restart = (config[ZONE_0]->GetRestart() || config[ZONE_0]->GetRestart_Flow()); | ||
| mixingplane = config[ZONE_0]->GetBoolMixingPlaneInterface(); | ||
| bool discrete_adjoint = config[ZONE_0]->GetDiscrete_Adjoint(); |
| config_container[donorZone], | ||
| config_container[targetZone]); | ||
| break; | ||
| // const auto nMarkerInt = config_container[donorZone]->GetnMarker_MixingPlaneInterface() / 2; |
Check notice
Code scanning / CodeQL
Commented-out code
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 days ago
The most appropriate fix is to remove the commented-out code block at lines 604-613, since it is not being used and no context suggests it should be retained. This will reduce distraction and prevent future confusion. There is no indication that the code should be reinstated, and since similar functionality appears to already exist in the BroadcastData_MixingPlane call, deleting the commented block is safe and will not harm functionality.
- Remove lines 604–613 in
SU2_CFD/src/drivers/CMultizoneDriver.cpp. - No imports, methods, or redefinitions are needed.
| @@ -601,16 +601,7 @@ | ||
| config_container[donorZone], | ||
| config_container[targetZone]); | ||
| break; | ||
| // const auto nMarkerInt = config_container[donorZone]->GetnMarker_MixingPlaneInterface() / 2; | ||
|
|
||
| // /*--- Transfer the average value from the donorZone to the targetZone ---*/ | ||
| // /*--- Loops over the mixing planes defined in the config file to find the correct mixing plane for the donor-target combination ---*/ | ||
| // for (auto iMarkerInt = 1; iMarkerInt <= nMarkerInt; iMarkerInt++) { | ||
| // interface_container[donorZone][targetZone]->AllgatherAverage(solver_container[donorZone][INST_0][MESH_0][FLOW_SOL],solver_container[targetZone][INST_0][MESH_0][FLOW_SOL], | ||
| // geometry_container[donorZone][INST_0][MESH_0],geometry_container[targetZone][INST_0][MESH_0], | ||
| // config_container[donorZone], config_container[targetZone], iMarkerInt ); | ||
| // } | ||
| // break; | ||
| } | ||
| default: | ||
| if(rank == MASTER_NODE) |
| for (iDim = 0; iDim < nDim; iDim++) | ||
| ProjVelocity_i += Velocity_i[iDim]*UnitNormal[iDim]; | ||
|
|
||
| auto nDonorSpan = GetnMixingStates(val_marker, iSpan); |
Check notice
Code scanning / CodeQL
Unused local variable
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 days ago
To fix this problem, simply remove the unused local variable declaration for nDonorSpan at line 5584 from SU2_CFD/src/solvers/CEulerSolver.cpp. This involves deleting the line with the assignment. As no other code depends on this variable, this change is safe and does not alter any functionality or logic elsewhere. No imports, new methods, or definitions are necessary.
| @@ -5581,7 +5581,6 @@ | ||
| for (iDim = 0; iDim < nDim; iDim++) | ||
| ProjVelocity_i += Velocity_i[iDim]*UnitNormal[iDim]; | ||
|
|
||
| auto nDonorSpan = GetnMixingStates(val_marker, iSpan); | ||
| su2double donorAverages[8] = {0.0}; | ||
| for (auto iVar = 0u; iVar < 8; iVar++) { | ||
| donorAverages[iVar] = GetMixingState(val_marker, iSpan, iVar); |
SU2_CFD/src/solvers/CEulerSolver.cpp
Outdated
|
|
||
| auto nDonorSpan = GetnMixingStates(val_marker, iSpan); | ||
| su2double donorAverages[8] = {0.0}; | ||
| for (auto iVar = 0u; iVar < 8; iVar++) { |
Check notice
Code scanning / CodeQL
Declaration hides variable
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 days ago
To fix the problem, we should rename the inner loop variable used at line 5586 (for (auto iVar = 0u; iVar < 8; iVar++)) to another unique name that does not shadow iVar declared at line 5477 in the function scope. The best approach is to pick a descriptive name such as mixVar, donorVar, or simply jVar, consistent with local naming convention.
Affected code region: SU2_CFD/src/solvers/CEulerSolver.cpp, lines around 5586-5588. We'll need to:
- Change
for (auto iVar = 0u; iVar < 8; iVar++) {to use something likefor (auto mixVar = 0u; mixVar < 8; mixVar++) { - Change all references to
iVarin that loop's body to use the new name:donorAverages[iVar] = GetMixingState(val_marker, iSpan, iVar);becomesdonorAverages[mixVar] = GetMixingState(val_marker, iSpan, mixVar);
No imports or other changes are needed. Just update the index name in the local loop and its uses.
-
Copy modified lines R5586-R5587
| @@ -5583,8 +5583,8 @@ | ||
|
|
||
| auto nDonorSpan = GetnMixingStates(val_marker, iSpan); | ||
| su2double donorAverages[8] = {0.0}; | ||
| for (auto iVar = 0u; iVar < 8; iVar++) { | ||
| donorAverages[iVar] = GetMixingState(val_marker, iSpan, iVar); | ||
| for (auto mixVar = 0u; mixVar < 8; mixVar++) { | ||
| donorAverages[mixVar] = GetMixingState(val_marker, iSpan, mixVar); | ||
| } | ||
| const auto ExtAverageDensity = donorAverages[0]; | ||
| const auto ExtAveragePressure = donorAverages[1]; |
SU2_CFD/src/solvers/CEulerSolver.cpp
Outdated
| switch (config->GetKind_Data_Giles(Marker_Tag)){ | ||
| case MIXING_IN: case MIXING_IN_1D: case MIXING_OUT: case MIXING_OUT_1D: | ||
| nDonorSpan = GetnMixingStates(val_marker, iSpan); | ||
| for (auto iVar = 0u; iVar < 8; iVar++) { |
Check notice
Code scanning / CodeQL
Declaration hides variable
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 days ago
To fix the issue, we should rename the loop variable iVar on line 6289 to something distinctive, such as mixVar, so it does not shadow the previously declared function-scope variable iVar (from line 6170). This only requires changes within the for loop block associated with donor averages (lines 6289–6291).
Steps:
- Change the loop variable name in the for-loop header and update its references within the loop body accordingly.
- No need to change anything else, as this variable is only used for donor averages.
This change occurs only in the function CEulerSolver::BC_Giles within SU2_CFD/src/solvers/CEulerSolver.cpp, specifically between approximately lines 6289–6291.
-
Copy modified lines R6289-R6290
| @@ -6286,8 +6286,8 @@ | ||
| switch (config->GetKind_Data_Giles(Marker_Tag)){ | ||
| case MIXING_IN: case MIXING_IN_1D: case MIXING_OUT: case MIXING_OUT_1D: | ||
| nDonorSpan = GetnMixingStates(val_marker, iSpan); | ||
| for (auto iVar = 0u; iVar < 8; iVar++) { | ||
| donorAverages[iVar] = GetMixingState(val_marker, iSpan, iVar); | ||
| for (auto mixVar = 0u; mixVar < 8; mixVar++) { | ||
| donorAverages[mixVar] = GetMixingState(val_marker, iSpan, mixVar); | ||
| } | ||
| ExtAverageDensity = donorAverages[0]; | ||
| ExtAveragePressure = donorAverages[1]; |
| for (auto iSpan = 0u; iSpan < nSpanWiseSections ; iSpan++){ | ||
|
|
||
| su2double extAverageNu = solver_container[FLOW_SOL]->GetExtAverageNu(val_marker, iSpan); | ||
| auto nDonorSpan = solver_container[FLOW_SOL]->GetnMixingStates(val_marker, iSpan); |
Check notice
Code scanning / CodeQL
Unused local variable
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 days ago
To fix the issue, remove the declaration of the unused local variable nDonorSpan from line 1078 in SU2_CFD/src/solvers/CTurbSASolver.cpp.
- This improves readability and avoids a potential source of confusion or errors.
- Only remove the declaration itself—do not touch any other lines.
- No imports or further code changes are necessary, since the variable is not used or required elsewhere in the displayed code.
| @@ -1075,7 +1075,6 @@ | ||
| /*--- Loop over all the vertices on this boundary marker ---*/ | ||
| for (auto iSpan = 0u; iSpan < nSpanWiseSections ; iSpan++){ | ||
|
|
||
| auto nDonorSpan = solver_container[FLOW_SOL]->GetnMixingStates(val_marker, iSpan); | ||
| const auto extAverageNu = solver_container[FLOW_SOL]->GetMixingState(val_marker, iSpan, 5); | ||
|
|
||
| // su2double extAverageNu = solver_container[FLOW_SOL]->GetExtAverageNu(val_marker, iSpan); |
| auto nDonorSpan = solver_container[FLOW_SOL]->GetnMixingStates(val_marker, iSpan); | ||
| const auto extAverageNu = solver_container[FLOW_SOL]->GetMixingState(val_marker, iSpan, 5); | ||
|
|
||
| // su2double extAverageNu = solver_container[FLOW_SOL]->GetExtAverageNu(val_marker, iSpan); |
Check notice
Code scanning / CodeQL
Commented-out code
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 days ago
To address this commented-out code issue, the best course of action is to remove the commented-out code line entirely, as it appears to be a remnant from previous code iterations and does not serve a documentation or explanatory purpose. No changes to the implementation or functionality are required; simply deleting line 1081 from SU2_CFD/src/solvers/CTurbSASolver.cpp will resolve the issue and improve code clarity.
| @@ -1078,7 +1078,6 @@ | ||
| auto nDonorSpan = solver_container[FLOW_SOL]->GetnMixingStates(val_marker, iSpan); | ||
| const auto extAverageNu = solver_container[FLOW_SOL]->GetMixingState(val_marker, iSpan, 5); | ||
|
|
||
| // su2double extAverageNu = solver_container[FLOW_SOL]->GetExtAverageNu(val_marker, iSpan); | ||
|
|
||
| /*--- Loop over all the vertices on this boundary marker ---*/ | ||
|
|
|
|
||
| su2double extAverageKine = solver_container[FLOW_SOL]->GetExtAverageKine(val_marker, iSpan); | ||
| su2double extAverageOmega = solver_container[FLOW_SOL]->GetExtAverageOmega(val_marker, iSpan); | ||
| auto nDonorSpan = solver_container[FLOW_SOL]->GetnMixingStates(val_marker, iSpan); |
Check notice
Code scanning / CodeQL
Unused local variable
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 days ago
To fix this problem, we should remove the unused local variable nDonorSpan from the function CTurbSSTSolver::BC_Inlet_MixingPlane.
- Specifically, delete or comment out the line that declares and assigns
nDonorSpan:auto nDonorSpan = solver_container[FLOW_SOL]->GetnMixingStates(val_marker, iSpan);
- This change is limited to a single line (line 806) within the function, and it will not affect any logic or required functionality because the variable is not accessed after being declared.
- No new imports, methods, or definitions are needed.
| @@ -803,7 +803,6 @@ | ||
|
|
||
| for (auto iSpan = 0u; iSpan < nSpanWiseSections ; iSpan++){ | ||
|
|
||
| auto nDonorSpan = solver_container[FLOW_SOL]->GetnMixingStates(val_marker, iSpan); | ||
| const auto extAverageKine = solver_container[FLOW_SOL]->GetMixingState(val_marker, iSpan, 6); | ||
| const auto extAverageOmega = solver_container[FLOW_SOL]->GetMixingState(val_marker, iSpan, 7); | ||
| // su2double extAverageKine = solver_container[FLOW_SOL]->GetExtAverageKine(val_marker, iSpan); |
| // su2double extAverageKine = solver_container[FLOW_SOL]->GetExtAverageKine(val_marker, iSpan); | ||
| // su2double extAverageOmega = solver_container[FLOW_SOL]->GetExtAverageOmega(val_marker, iSpan); |
Check notice
Code scanning / CodeQL
Commented-out code
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 days ago
To fix the issue, we should remove the commented-out code lines, specifically lines 809 and 810 in SU2_CFD/src/solvers/CTurbSSTSolver.cpp. These lines don't serve as documentation, so they should be deleted, not reformatted. Only remove the lines that are exclusively commented-out code, making sure to retain surrounding code and comments that are not code. No imports, methods, or definitions need to be added or changed; it's a simple removal within the file.
| @@ -806,8 +806,6 @@ | ||
| auto nDonorSpan = solver_container[FLOW_SOL]->GetnMixingStates(val_marker, iSpan); | ||
| const auto extAverageKine = solver_container[FLOW_SOL]->GetMixingState(val_marker, iSpan, 6); | ||
| const auto extAverageOmega = solver_container[FLOW_SOL]->GetMixingState(val_marker, iSpan, 7); | ||
| // su2double extAverageKine = solver_container[FLOW_SOL]->GetExtAverageKine(val_marker, iSpan); | ||
| // su2double extAverageOmega = solver_container[FLOW_SOL]->GetExtAverageOmega(val_marker, iSpan); | ||
| su2double solution_j[] = {extAverageKine, extAverageOmega}; | ||
|
|
||
| /*--- Loop over all the vertices on this boundary marker ---*/ |
…m/su2code/SU2 into feature_mz_turbomachinery_adjoint
Proposed Changes
This is a cleaned up PR of the fixes needed for multizone adjoints for turbomachinery from the previous PR of @oleburghardt and I's work.
This hopefully is useable, so if you would like to test and report please feel free to contact me on Slack.
TODO:
Related Work
Now closed PR #2317
PR Checklist
Put an X by all that apply. You can fill this out after submitting the PR. If you have any questions, don't hesitate to ask! We want to help. These are a guide for you to know what the reviewers will be looking for in your contribution.
pre-commit run --allto format old commits.