code-saturne / code_saturne

code_saturne public mirror
https://www.code-saturne.org
GNU General Public License v2.0
215 stars 80 forks source link

NVD/TVD Scheme does not need Beta limiter or Slope Test #103

Closed paspro closed 7 months ago

paspro commented 1 year ago

In file cs_convection_diffusion.c, line 7430 there is the following check:

  /* --> Flux with no slope test or Min/Max Beta limiter
    ====================================================*/

  }
  else if (isstpp == 1 || isstpp == 2) {

    if (ischcp < 0 || ischcp > 2) {
      bft_error(__FILE__, __LINE__, 0,
                _("invalid value of ischcv"));
    }

This check produces an error if a second-order accurate scheme is enabled but neither the Beta limiter nor the Slope Test options are enabled. This is correct with the exception of a NVD/TVD scheme (ischcp = 4) in which case there is no need for any of these options because the scheme is designed to produce monotone solutions on its own. So, I think that this should be modified to the following:

  /* --> Flux with no slope test or Min/Max Beta limiter
    ====================================================*/

  }
  else if (isstpp == 1 || isstpp == 2) {

    if (ischcp < 0 || (ischcp > 2 && ischcp != 4)) {
      bft_error(__FILE__, __LINE__, 0,
                _("invalid value of ischcv"));
    }
YvanFournier commented 1 year ago

Thanks for the report. I'll check (always happy to simplify tests).

paspro commented 1 year ago

Any progress on this issue? The fix is very simple.

erwanlecoupanec commented 1 year ago

Hi Panos,

Which version of code_saturne are you playing with ? These lines of code have been changed in 7.0 already.

I'm surprised by your interpretation of these lines anyway. From memory, before 7.0, you could not use the beta limiter on top of a TVD/NVD scheme. From 7.0 on, it has become possible and we use it on top of the scheme we use for VOF (CICSAM-M that you implemented) to ensure more robustness.

erwanlecoupanec commented 1 year ago

In a nutshell, with 7.0, you should of course be able to use a TVD/NVD scheme without slope test nor beta limiter, but it was already true in 6.0.

Regards, Erwan

paspro commented 1 year ago

Hi Erwan,

In the master version, the line of code I am talking about is in line 7504. In version 7.3 it is in line 7536 and in version 8.0 it is in line 7430. The problem is that this check does not allow the user to select a TVD/NVD scheme without either the Beta limiter or the slope limiter.

A TVD/NVD scheme computes limited fluxes for each cell face in separate using information from the current cell and the cell across the face. It does not require limited gradients because in that case extra diffusion will be introduced in the solution. The option of limited gradients makes sense in the case of using the SOLU scheme in which case it guarantees that the face values extrapolated using gradients from both cells are indeed bounded so that no blending is required. The same applies for the case of having some type of Riemann solver to compute the fluxes for compressible flows. A TVD/NVD scheme is capable of producing monotone solutions with no need for some special treatment for the gradients. At least, the code should offer the option to the user to enable such a scheme with or without a gradient or slope limiter and not force its use.

I am working on a project which involves the solution of multiple scalar equations and I have made the appropriate modification to the code as explained in the initial post and I have no issues with stability, convergence or monotonicity.

erwanlecoupanec commented 1 year ago

Ok. Thanks for the version. I did not check the line you indicated previously since there was no version associated.

The piece of code you mention is in cs_convection_diffusion_thermal. The TVD/NVD scheme have not been tested for the thermal scalar I believe, though the developments made in 7.0 were supposed to allow it. So thanks for the bug correction. The piece of code should be similar to what has been implemented at line 3606 for standard scalar. It will be fixed soon. I guess the scalar you tried to compute the balance of was a thermal scalar then.

What I said above was for the void fraction using the VOF method. The balance is in that case computed with cs_convection_diffusion_scalar and the check of ischtp is correctly done there.

Thanks, Erwan