first release of the turbomachinery capabilities #413

salvovitale · 2017-07-13T16:34:03Z

This pull request enlarges the capability of SU2 to simulate internal flows in turbomachinery. The flow solver is now capable of performing 3D steady multi-zone simulations via a 3D Non-Reflecting Mixing Plane. The implementation is based on the work done by Saxer and Giles (available at https://doi.org/10.2514/3.23618). To adapt the implementation of Saxer and Giles to an unstructured vertex-based solver such as SU2, a new data structure, called CTurboVertex, was created for the inflow and outflow boundaries. The CTurboVertex class is a child of the class CVertex; thus, it maintains all the standard vertex properties. However, more than a standard vertex, the turboVertex posses:

the possibility of accessing the inflow and outflow boundaries on a span-wise manner (to impose different values of the mixing-plane condition along the blade-height) and on a pitch-wise ordered manner (to compute the Spatial Fourier Transformation of the outgoing characteristics at the boundary for the non-reflectivity).
embedded information on what kind of turbomachinery architecture is being simulated, so that the values of velocity at the boundaries can be imposed using the correct frame of reference.

So in a nutshell the flow solver is now capable of:

simulating single 2D/3D turbomachinery blades using 1D characteristic-based BC or Non reflecting BC
simulating multi-stage 2D/3D turbomachinery via a Mixing-Plane interface, including also the mixing of the turbulent quantities
handling any kind of turbomachinery architecture (AXIAL, CENTRIFUGAL, CENTRIPETAL and mixed such as CENTRIPETAL_AXIAL or AXIAL_CENTRIFUGAL)
selecting 2 different kind of interpolation methods for the mixing-plane interface: LINEAR_INTERPOLATION and NEAREST_NEIGHBOR
handling an unlimited number of blades/stages —> (zones)
simulating turbomachinery which works on the NICFD regime such as supercritical CO2 compressors or Organic Rankine Cycle turbines.
printing history_x.dat file for multi-zone convergence monitoring
printing some post-processing thermodynamic, kinematic and geometric quantities in some files inside a TURBOMACHINERY folder

The limitations of the turbomachinery capabilities are:

the axis of rotation should be always the Z one
MGL acceleration technique is not working because of some un-resolved issues with the periodic BC.

This pull request introduce also some limited design capability for turbomachinery applications. Using the AD tool, it is now available on the suite also a single zone discrete adjoint to compute the sensitivity for 2D/3D turbomachinery blade with respect to certain performance parameters (ENTROPY_GENERATION, TOTAL_PRESSURE_LOSS, KINETIC_ENERGY_LOSS, ecc.). However, the optimization script only works for 2D blades since the FFD and the mesh deformation routines need still to be adapted to handle 3D blades (especially for problem on the deformation of the hub and shroud surfaces).

…mance not yet parallel

…river and AdjTurboDriver

…nto feature_NRBC3D

ghost · 2017-07-19T01:52:38Z

Common/include/dual_grid_structure.hpp

+ * \author S. Vitale
+ * \version 5.0.0 "Raven"
+ */
+class CTurboVertex : public CVertex {


What is the main reason for not using CVertex?

As I said in the description of the pull request, to impose Non reflecting BC and Mixing plane you need to access the boundary information on a span-wise and pitch wise ordered manner. Moreover, you need to convert the velocities in different frame of references w.r.t. the turbomachinery architecture you are simulating. The turboVertex takes care of all of these needs.

Perfect. thanks!

ghost · 2017-07-19T01:53:08Z

Common/include/option_structure.hpp

@@ -625,13 +625,15 @@ enum ENUM_LIMITER {
  VENKATAKRISHNAN = 0,	/*!< \brief Slope limiter using Venkatakrisnan method. */
  BARTH_JESPERSEN = 1,  /*!< \brief Slope limiter using Barth-Jespersen method. */
  SHARP_EDGES = 2,       /*!< \brief Slope limiter using sharp edges. */
-  SOLID_WALL_DISTANCE = 3       /*!< \brief Slope limiter using wall distance. */
+  SOLID_WALL_DISTANCE = 3,       /*!< \brief Slope limiter using wall distance. */
+	VAN_ALBADA = 4         /*!< \brief Slope limiter using wall distance. */


Could you please update the description. Have your experimented with this limiter? I think Venkat's limiter was an improvement over the Van Albada. Anyway, it is great to count with another limiter implementation.

I have a bunch of test-cases in which the Van Albada works much better than the Venkatakrishnan. Usually what happens is that the residuals stall with the Venka, but instead
they keep going down with the Van Albada.

ghost · 2017-07-19T01:53:20Z

Common/include/option_structure.hpp


 public:
-	  COptionNRBC(string option_field_name, unsigned short & nMarker_NRBC, string* & Marker_NRBC, unsigned short* & option_field,


NR stands for Non-reflecting? isn't it? Maybe we should be a little more explicit and use something like COptionNonReflectingBC

I have changed everything from NRBC --> Giles_BC.

Mike is going to be happy :-)

ghost · 2017-07-19T01:53:26Z

Common/include/option_structure.hpp

+        return badValue(option_value, "NRBC", this->name);
+      }
+      istringstream ss_6th(option_value[9*i + 7]);
+			if (!(ss_6th >> this->relfac1[i])) {


Is there something wrong with the number os spaces?

ghost · 2017-07-19T01:53:44Z

Common/src/config_structure.cpp

-  addNRBCOption("MARKER_NRBC", nMarker_NRBC, Marker_NRBC, Kind_Data_NRBC, NRBC_Map, NRBC_Var1, NRBC_Var2, NRBC_FlowDir);
-  /*!\brief MIXING_PROCESS_TYPE \n DESCRIPTION: types of mixing process for averaging quantities at the boundaries.
+  /*!\brief MARKER_NRBC \n DESCRIPTION: Riemann boundary marker(s) with the following formats, a unit vector. */
+  addNRBCOption("MARKER_NRBC", nMarker_NRBC, Marker_NRBC, Kind_Data_NRBC, NRBC_Map, NRBC_Var1, NRBC_Var2, NRBC_FlowDir, RelaxFactorAverage, RelaxFactorFourier);


Could you please update the description

ghost · 2017-07-19T01:54:18Z

Common/src/config_structure.cpp

  /*!\brief MARKER_MIXINGPLANE \n DESCRIPTION: Identify the boundaries in which the mixing plane is applied. \ingroup Config*/
-  addTurboPerfOption("MARKER_TURBO_PERFORMANCE", nMarker_TurboPerf, Marker_TurboBoundIn, Marker_TurboBoundOut, Kind_TurboPerformance, TurboPerformance_Map);
+  addTurboPerfOption("MARKER_TURBOMACHINERY", nMarker_Turbomachinery, Marker_TurboBoundIn, Marker_TurboBoundOut);


Please update the description

ghost · 2017-07-19T01:57:58Z

Common/src/geometry_structure.cpp

+
+#ifdef HAVE_MPI
+
+    MyTotalArea 


Is there any change in CPhysicalGeometry::SetCoord_CG? It looks like that the diff tool is not working properly

No I did not touch that routine. I think the diff tool is confused.

ghost · 2017-07-19T01:58:35Z

Common/src/geometry_structure.cpp

+
+#ifdef HAVE_MPI
+
+    MyTotalArea 


Same comment here, are you modifying CPhysicalGeometry::SetBoundControlVolume?

ghost · 2017-07-19T01:59:48Z

Common/src/grid_movement_structure.cpp


        /*--- Solve the linear system (GMRES with restart) ---*/

-      case RESTARTED_FGMRES:


@as before, is there any relevant change in this subroutine? If there is no change, we should check that the diff tool is working properly.

Personally, I did not change this routine. @talbring did you modify this one?

the whole switch statement is wrapped in an if condition now, thats all (that is needed for multizone deformations). I dont know why github has problems even here ...

Thanks @talbring for the prompt answer!

ghost · 2017-07-19T02:01:35Z

SU2_CFD/include/output_structure.hpp

@@ -47,7 +47,7 @@
 #include <time.h>
 #include <fstream>

-#include "solver_structure.hpp"
+//#include "solver_structure.hpp"


What is the reason for commenting out this line?

I was experimenting some stuff. In truth this header file does not need to be included. For the moment, I have included again.

ghost · 2017-07-19T02:02:18Z

SU2_CFD/include/solver_structure.hpp

-  virtual void BC_Inlet(CGeometry *geometry, CSolver **solver_container, CNumerics *conv_numerics, CNumerics *visc_numerics,
-                        CConfig *config, unsigned short val_marker);
-
+  virtual void PreprocessBC_NonReflecting(CGeometry *geometry, CConfig *config, CNumerics *conv_numerics, unsigned short marker_flag);


I like the fact that you are using NonREflecting instead of NR

Now everything is BC_Giles so this one becomes PreprocessBC_Giles.

ghost · 2017-07-19T02:03:22Z

SU2_CFD/include/solver_structure.hpp

@@ -947,7 +944,8 @@ class CSolver {
   * \param[in] config - Definition of the particular problem.
   * \param[in] val_marker - Surface marker where the boundary condition is applied.
   */
-  virtual void BC_Engine_Inflow(CGeometry *geometry, CSolver **solver_container, CNumerics *conv_numerics, CNumerics *visc_numerics, CConfig *config, unsigned short val_marker);
+  virtual void BC_Supersonic_Inlet(CGeometry *geometry, CSolver **solver_container,


Yep... the diff tool is confused.

very confused! as @talbring suggested, especially for the solver_direct_mean.cpp is better to use another difftoll such as, for example, MELD

For the time being... no problem... let's try to do it different next time. Thanks!

ghost

Dear Salvo, this is a VERY important contribution to SU2. It is very well done and we'll approve it as soon as possible so we can start testing the develop branch after this huge amount of changes.

Obviously, in the future, we should try to divide the contributions in small pull request... otherwise the reviewing process is very complicated. And, we will have some issues with the capabilities of the develop branch after the merging.

Keep up the great work!

Thanks for your generosity sharing your work,
Francisco

PS.- @talbring if we accept this pull request with thousands of changes we should probably take a look at @JSmith36 pull request (I'll review the implementation today). From now on we should try to encourage small pull request.

economon · 2017-07-19T06:53:52Z

Salvo: thanks for the pull request, very exciting stuff! I will also add a review soon, but perhaps I will let you comment on some of the review by @fpalacios first.

talbring · 2017-07-19T07:01:28Z

@fpalacios, as you already noticed github has problems viewing the diff of solver_direct_mean.cpp correctly. Most of the changes there are just additions. I can suggest to do a local checkout and use a proper difftool.

salvovitale · 2017-07-19T10:24:40Z

Dear @fpalacios ,
thank you very much for taking the time to go trough the review process of the pull request.
I 've made all the change you requested.

Regarding splitting in smaller pull requests I agree. However, I believe that usually there are 2 kind of pull requests:

improving existing features
creating new features

While for the type 1 I think is possible to keep pull requests small, for the type 2 this is not always the case. In any case I agree we should always keep the review process in mind.

best

Salvo

talbring · 2017-07-19T10:31:43Z

Just to clarify things, I am not against pull requests with a lot of changes, @fpalacios. For me the problem in #412 is simply the fact that everything is included in one commit.

LaSerpe · 2017-07-19T10:38:39Z

SU2_CFD/src/driver_structure.cpp

+
+}
+
+void CTurbomachineryDriver::SetMixingPlane(unsigned short donorZone){


Perhaps we could think about moving the mixing plane into the interpolator_class.
In this way we'll have all the routines that manage the communication between zones in the same place.

Indeed, in fact the Mixing-Plane interface is already implemented within the Transfer structure, in which we exchange info between zones. However, I did not use the the interpolator_class because it does not occur an interpolation between the nodes of the interfaces.

I see, so maybe we should change the name of the class to something else.
Indeed "interpolation" (in a strictly mathematical sense) doesn't actually occur even for the nearest neighbour approach (zones are loosely coupled and the interface is basically treated as a non-homogeneous BC).
Having all the possible way of communicating between zones in the same src would make the code much more clear, and possibly we could avoid creating the new driver class just by modifying the fluid driver a little bit.
By the way, we may do this in a later pull request (so nevermind) I'm just trying to figure out if that would make sense.

Yeah I see your point. However the new turbo driver is a child of the new FluidDriver, and only few routines are overwritten:

the Run() routine because i need to use the iterator container in a slightly different way

the Monitor() routine in which I implemented ad hoc methods for ramping up the outlet pressure and the rotational speed to facilitate the convergence of multi-stage simulations.

Indeed I could have implemented on the CFluidDriver, but there are two reason why I did not do it:

I wanted to avoid many if statements to customize the driver

(and more important) a clear structure of the turbo Driver also facilitate the implementation of his adjoint counterpart.

I' m pretty sure that when the structure is there we can always improve the code with future pull requests, as we have been doing so far. Anyway thanks for your valuable comments Giulio.

regards

Salvo

Very interesting discussion. Please, after this pull request, do your best to revisit the issue to be sure that we have the best solution possible. Thanks

ghost · 2017-07-22T22:56:21Z

Thanks @salvovitale for this contribution, the open-source community is going to appreciate a lot this new capability.

In fact, I think it is a perfect timing to prepare a tutorial about turbo machinery in https://github.com/su2code/SU2/wiki/User-Tutorials

As you know, the tutorial is a really important step. Otherwise, for an average users, it is difficult to know that this awesome capability is available. And... a large number of users will help us to improve any implementation.

Best,
Francisco

salvovitale · 2017-07-23T11:01:19Z

Dear @fpalacios
thank you very much for merging this in.

I agree that one or two tutorials are fundamental to let the users exploit the new capabilities. I will certainly count on the help of the community to make this new feature as much reliable and general as possible.

regards

Salvo

salvovitale and others added 30 commits August 12, 2016 16:59

implement automatic computation of span-wise sections

1a55320

first version working with automatic spanwise computation

c26d95a

implement angular pitch distance for turbovertex

0896d1f

implement GLOBAL condition PT for non reflecting BC

1d9cb9a

fix conflct after merge with turbPerf2D

0968bdf

reststruturing code WIP1 (not working)

2ce4ca4

move turboperformance in COutput Class

2ce8669

almost get to compile

3a033fe

first compiled version not test yet

7cd35b6

solver single zone working, inflow outflow quantities for turboperfor…

a109cfb

…mance not yet parallel

implement GatherInOutAverageValues and some cleaning

c73be36

initilize some quantities in the Euler constructor

7bb812c

add deallocate turbomachinery quantities CEulerSolver Destructor

95f3d75

implement inflow and outflow geo turbo qts needed for turboperformance

e3950ee

compile but still with a segm fault

d846bbc

Fixed some warnings and memory leaks.

ee06413

adjoint solver working for single zone. (only entropy gen at the moment)

f815c20

Further memory leaks fixed.

dcf4b94

Added routine to compute obj. function

da1788a

implement new turbo drivers for direct and discr adj solver

dafbec6

comments and small change

604d7b2

fix bug area out on Compute TurboPerformance

ea4465f

implement values from BC for monitoring and modify constructor TurboD…

8999e53

…river and AdjTurboDriver

compute some turbo perf quantities

ae71620

inset monitoring Ttotal Ptotal for BC_inlet

802b524

fix inlet and outlet BC video monitoring

9d3f3bb

fix output video for mixing-plane convergence

e8de598

implement computation of mach numbers for turboperformance

6c6ae4a

turbo vertex for centrifugal compressor

a4c6759

Merge branch 'feature_NRBC3D' of https://github.com/salvovitale/SU2 i…

7a58106

…nto feature_NRBC3D

ghost reviewed Jul 19, 2017

View reviewed changes

ghost suggested changes Jul 19, 2017

View reviewed changes

salvovitale added 5 commits July 19, 2017 10:26

update comments for VAN ALBADA limiter

a8cae3a

change name NRBC --> Giles_BC

708689d

update some comments

1ca1e4b

update other comments

2c88203

remove commented header file on the output structure

95449f9

LaSerpe reviewed Jul 19, 2017

View reviewed changes

salvovitale added 2 commits July 19, 2017 12:57

update config template [ci skip]

6dea3bf

correct APU test case config file

e3e8033

ghost approved these changes Jul 22, 2017

View reviewed changes

ghost merged commit 6bac912 into develop Jul 22, 2017

salvovitale deleted the feature_turbomachinery branch August 9, 2017 14:34

This pull request was closed.


		public:
		COptionNRBC(string option_field_name, unsigned short & nMarker_NRBC, string* & Marker_NRBC, unsigned short* & option_field,


		/--- Solve the linear system (GMRES with restart) ---/

		case RESTARTED_FGMRES:


		}

		void CTurbomachineryDriver::SetMixingPlane(unsigned short donorZone){

first release of the turbomachinery capabilities #413

first release of the turbomachinery capabilities #413

Conversation

salvovitale commented Jul 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost Jul 19, 2017 • edited by ghost Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

salvovitale Jul 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost left a comment

Choose a reason for hiding this comment

economon commented Jul 19, 2017

talbring commented Jul 19, 2017

salvovitale commented Jul 19, 2017

talbring commented Jul 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Jul 22, 2017

salvovitale commented Jul 23, 2017 • edited Loading

ghost Jul 19, 2017 •

edited by ghost

Loading

salvovitale Jul 19, 2017 •

edited

Loading

salvovitale commented Jul 23, 2017 •

edited

Loading