Remove reduntant constructor in fluid

[timofeymukha]

• Removes the _uvwm constructor. The code inside it overlapped with what was in the _all, and both called common, so the contents of _uvw could just be moved to common.
• The _all essentially now deals with pressure. I think this should in principle be in fluid_pnpn and it would be most reasonable to move it there in a subsequent PR. E.g. the space of pressure may be different.
• Moved the code for the output_bdry to a separate subroutine

• I moved most of the logging to the end of _all. This now resulted in that the log of GS comes before it says "Fluid"

   --------Gather-Scatter--------  
    Comm         :          MPI
    Avg. internal:       290672
    Avg. external:            0
    Backend      :          std
  
    --------Gather-Scatter--------  
    Comm         :          MPI
    Avg. internal:        16336
    Avg. external:            0
    Backend      :          std
  
    --------Gather-Scatter--------  
    Comm         :          MPI
    Avg. internal:       109424
    Avg. external:            0
    Backend      :          std
  
    --------Gather-Scatter--------  
    Comm         :          MPI
    Avg. internal:       560080
    Avg. external:            0
    Backend      :          std
  
    --------Gather-Scatter--------  
    Comm         :          MPI
    Avg. internal:       290672
    Avg. external:            0
    Backend      :          std
  
    ------------Fluid-------------  
    Type       : Modular (Pn/Pn)
    Ksp prs.   : (gmres, hsmg)
     `-abs tol : 1.000000E-07
    Ksp vel.   : (cg, jacobi)
     `-abs tol : 1.000000E-07
    rho        : 1.000000E+00
    mu         : 7.142857E-04
    Dealias    : T
    Save bdry  : T

  Since we mostly reuse the GS I think this is fine, but I can move the header back if needed.

[timofeymukha]

I would argue for keeping the Fluid section such that it still covers all the initialisations, such that logging, error/warning messages are correctly indented in the log file

I have update now to start the Fluid section as before. I've also changed lx to poly order in the log so that it is consistent with the user file. I've also added output of dof%size(), I think this is quite useful even though it is a derivatve of poly order and n elements.

      ------------Fluid-------------  
      Type       : Modular (Pn/Pn)
      Poly order : 5
      DoFs       : 441072

          --------Gather-Scatter--------  
          Comm         :          MPI
          Avg. internal:       290672
          Avg. external:            0
          Backend      :          std

          --------Gather-Scatter--------  
          Comm         :          MPI
          Avg. internal:        16336
          Avg. external:            0
          Backend      :          std

          --------Gather-Scatter--------  
          Comm         :          MPI
          Avg. internal:       109424
          Avg. external:            0
          Backend      :          std

          --------Gather-Scatter--------  
          Comm         :          MPI
          Avg. internal:       560080
          Avg. external:            0
          Backend      :          std

          --------Gather-Scatter--------  
          Comm         :          MPI
          Avg. internal:       290672
          Avg. external:            0
          Backend      :          std
      Ksp prs.   : (gmres, hsmg)
       `-abs tol : 1.000000E-07
      Ksp vel.   : (cg, jacobi)
       `-abs tol : 1.000000E-07
      rho        : 1.000000E+00
      mu         : 7.142857E-04
      Dealias    : T
      Save bdry  : T

[njansson]

I would argue for keeping the Fluid section such that it still covers all the initialisations, such that logging, error/warning messages are correctly indented in the log file

I have update now to start the Fluid section as before. I've also changed lx to poly order in the log so that it is consistent with the user file. I've also added output of dof%size(), I think this is quite useful even though it is a derivatve of poly order and n elements.

      ------------Fluid-------------  
      Type       : Modular (Pn/Pn)
      Poly order : 5
      DoFs       : 441072

          --------Gather-Scatter--------  
          Comm         :          MPI
          Avg. internal:       290672
          Avg. external:            0
          Backend      :          std

          --------Gather-Scatter--------  
          Comm         :          MPI
          Avg. internal:        16336
          Avg. external:            0
          Backend      :          std

          --------Gather-Scatter--------  
          Comm         :          MPI
          Avg. internal:       109424
          Avg. external:            0
          Backend      :          std

          --------Gather-Scatter--------  
          Comm         :          MPI
          Avg. internal:       560080
          Avg. external:            0
          Backend      :          std

          --------Gather-Scatter--------  
          Comm         :          MPI
          Avg. internal:       290672
          Avg. external:            0
          Backend      :          std
      Ksp prs.   : (gmres, hsmg)
       `-abs tol : 1.000000E-07
      Ksp vel.   : (cg, jacobi)
       `-abs tol : 1.000000E-07
      rho        : 1.000000E+00
      mu         : 7.142857E-04
      Dealias    : T
      Save bdry  : T

Great, but can't we keep the parameter listing together as we did before? I think it's a bit strange that e.g output from hsmg comes before we output which solver we have chosen, especially if we add some more logging from e.g. hsmg init

[timofeymukha]

I would argue for keeping the Fluid section such that it still covers all the initialisations, such that logging, error/warning messages are correctly indented in the log file

I have update now to start the Fluid section as before. I've also changed lx to poly order in the log so that it is consistent with the user file. I've also added output of dof%size(), I think this is quite useful even though it is a derivatve of poly order and n elements.

      ------------Fluid-------------  
      Type       : Modular (Pn/Pn)
      Poly order : 5
      DoFs       : 441072

          --------Gather-Scatter--------  
          Comm         :          MPI
          Avg. internal:       290672
          Avg. external:            0
          Backend      :          std

          --------Gather-Scatter--------  
          Comm         :          MPI
          Avg. internal:        16336
          Avg. external:            0
          Backend      :          std

          --------Gather-Scatter--------  
          Comm         :          MPI
          Avg. internal:       109424
          Avg. external:            0
          Backend      :          std

          --------Gather-Scatter--------  
          Comm         :          MPI
          Avg. internal:       560080
          Avg. external:            0
          Backend      :          std

          --------Gather-Scatter--------  
          Comm         :          MPI
          Avg. internal:       290672
          Avg. external:            0
          Backend      :          std
      Ksp prs.   : (gmres, hsmg)
       `-abs tol : 1.000000E-07
      Ksp vel.   : (cg, jacobi)
       `-abs tol : 1.000000E-07
      rho        : 1.000000E+00
      mu         : 7.142857E-04
      Dealias    : T
      Save bdry  : T

Great, but can't we keep the parameter listing together as we did before? I think it's a bit strange that e.g output from hsmg comes before we output which solver we have chosen, especially if we add some more logging from e.g. hsmg init

Ok, Ì see your point, how about having everything local then

      ------------Fluid-------------  
      Type       : Modular (Pn/Pn)
      Poly order : 5
      DoFs       : 441072
      rho        : 1.000000E+00
      mu         : 7.142857E-04
      Dealias    : T
      Save bdry  : T

          --------Gather-Scatter--------  
          Comm         :          MPI
          Avg. internal:       290672
          Avg. external:            0
          Backend      :          std
      Ksp vel.   : (cg, jacobi)
       `-abs tol : 1.000000E-07
      Ksp prs.   : (gmres, hsmg)
       `-abs tol : 1.000000E-07

          --------Gather-Scatter--------  
          Comm         :          MPI
          Avg. internal:        16336
          Avg. external:            0
          Backend      :          std

          --------Gather-Scatter--------  
          Comm         :          MPI
          Avg. internal:       109424
          Avg. external:            0
          Backend      :          std

          --------Gather-Scatter--------  
          Comm         :          MPI
          Avg. internal:       560080
          Avg. external:            0
          Backend      :          std

          --------Gather-Scatter--------  
          Comm         :          MPI
          Avg. internal:       290672
          Avg. external:            0
          Backend      :          std

The log output regarding the solvers is now generated when the solvers are initialized. This is consistent with the fact that we have flags for whether to do that: kspv_init and kspp_init. Previously, we would probe the case file for the setting of both solvers, even if they were potentially not used/initialized. I would also anticipate that the pressure solver will be logged for in the fluid_pnpn, since the pressure setup should be there.

I was actually blissfully unaware that we have so many gather-scatter in the log due to the pressure solver, so I think this is a good change logically.

[timofeymukha]

Fixed the closure of the material properties log section as bonus. @njansson how do you like the format above?

[njansson]

Fixed the closure of the material properties log section as bonus.

@njansson how do you like the format above?

Would it be possible to move the krylov info above the first gather scatter init?

If not I suggest we add a section called krylov or something to enclose it. Later I thing we should enclose the hsmg gs init in a section as well (in another pr)

[timofeymukha]

Fixed the closure of the material properties log section as bonus. @njansson how do you like the format above?

Would it be possible to move the krylov info above the first gather scatter init?

If not I suggest we add a section called krylov or something to enclose it. Later I thing we should enclose the hsmg gs init in a section as well (in another pr)

   ------------Fluid-------------  
   Type       : Modular (Pn/Pn)
   Poly order : 5
   DoFs       : 13824
   rho        : 1.000000E+00
   mu         : 7.142857E-04
   Dealias    : T
   Save bdry  : T

      --------Gather-Scatter--------  
      Comm         :          MPI
      Avg. internal:         3137
      Avg. external:         5946
      Backend      :          std

      -------Velocity solver--------  
      Type       : (cg, jacobi)
      Abs tol    : 1.000000E-07

      -------Pressure solver--------  
      Type       : (gmres, hsmg)
      Abs tol    : 1.000000E-07

          --------Gather-Scatter--------  
          Comm         :          MPI
          Avg. internal:           42
          Avg. external:          468
          Backend      :          std

          --------Gather-Scatter--------  
          Comm         :          MPI
          Avg. internal:          974
          Avg. external:         2444
          Backend      :          std

          --------Gather-Scatter--------  
          Comm         :          MPI
          Avg. internal:         6530
          Avg. external:        10972
          Backend      :          std

          --------Gather-Scatter--------  
          Comm         :          MPI
          Avg. internal:         3137
          Avg. external:         5946
          Backend      :          std

[njansson]

@timofeymukha something I thought of yesterday. Do we still have the possibility to init fluid_scheme with u,v,w (if e.g. we want to add a pnpn-2 solver)?

[timofeymukha]

@timofeymukha something I thought of yesterday. Do we still have the possibility to init fluid_scheme with u,v,w (if e.g. we want to add a pnpn-2 solver)?

So, that uvwconstructor called the _common one. And the additional stuff it did was also done in _all. So I just moved all the stuff in uvw to common. So, calling common now is essentially equivalent to calling uvw before. So I think the answer to your question is yes. I think down the line we can get away with only having 1 constructor -- which is currently common, and the pressure stuff (which is now in _all) will be handled in children: pnpn, pnpn-2 etc.

[timofeymukha]

@njansson @timfelle Are we happy?

[timofeymukha]

Is the Mac CI just acting up or?

I would really like to get this merged soon, so I can continue the work on the bcs.

[timfelle]

Is the Mac CI just acting up or?

I would really like to get this merged soon, so I can continue the work on the bcs.

Yes it died during the CI rework. Not exactly sure if i broke something or what happened. Some of the tests written in the pfunit framework seem to fail.

[timofeymukha]

@njansson-sama, take a look before it is out of sync with develop kudasai :D.