Skip to content
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

Standardized naming of the kernels #10

Closed
5 tasks
geriJana opened this issue Feb 13, 2024 · 6 comments
Closed
5 tasks

Standardized naming of the kernels #10

geriJana opened this issue Feb 13, 2024 · 6 comments
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@geriJana
Copy link

geriJana commented Feb 13, 2024

It seems that inconsistencies have been introduced into the kernel naming convention of the routines as well as some configuration options (see also #9).

In general

E.g. what does ... mean or when to use ...

  • generic
  • stencil suffix like d3q27

Decide on

  • When to put the stencil in the routine name
  • When to use generic
  • Which order to use for the routine name: e.g. scheme_regularization_stencil_generic or regularization_scheme_stencil_generic
  • Which order to use for configuration options

Correction routines

Besides, how should we name the correction routine bgk_HybridRecursiveRegularizedCorr ? It only differs in the blending parameter sigma but is now called for different configure options as listed below:

    case ('hrr_bgk_corrected', 'prr_bgk_corrected', 'rr_bgk_corrected')
      select case (trim(layout))
      case ('d3q27')
        compute => bgk_HybridRecursiveRegularizedCorr_d3q27
      case ('d3q19')
        compute => bgk_HybridRecursiveRegularizedCorr_d3q19
      case ('d2q9')
        compute => bgk_HybridRecursiveRegularizedCorr_d2q9

Suggestion:
bgk_blendedRecursiveRegularizedCorrected
Please note also the change of word ordering and removed trailing capital later in routine name.

Decide on

  • routine name for correction routines

Control

Once the naming convention is set for the routine names as well as the configuration options, we should check all routines for discrepancies and to identify need for renaming.

Harald asked @KannanMasilamani to decide on the names.

@geriJana geriJana added the invalid This doesn't seem right label Feb 13, 2024
@haraldkl
Copy link
Member

Thanks @geriJana. This mostly affects source/init/mus_initFluid_module.f90, I think.

@KannanMasilamani
Copy link
Contributor

KannanMasilamani commented Apr 16, 2024

To handle several variants of the scheme relaxations, we have decided to extend the relaxation option in scheme identify table as a table with additional variant option.

identify = {
  kind = 'fluid',
  relaxation = { 
    name = 'bgk', 
    variant = 'improved'
  },
  layout = 'd3q27'
}

If that variant requires addditional parameters then they should be inside the relaxation table like

identify = {
  kind = 'fluid',
  relaxation = {
    name = 'bgk', 
    variant = 'hybrid_recursive_regularized', 
    regularization_omega=0.98
  },
  layout = 'd3q27'
}

Standard variant which is a default can be defined by any of these forms:

  • relaxation='bgk'
  • relaxation={ name = 'bgk'}
  • relaxation={name='bgk', variant='standard'}

Here is the new naming convection of the kernels.
mus_advRel_k<kind>_r<relaxation>_v<variant>_l<layout>
Examples:

  • mus_advRel_kFluid_rBGK_vStd_lD3Q19
  • mus_advRel_kFluidIncomp_rBGK_vStd_lD3Q19
  • mus_advRel_kFluid_rBGK_vHRR_lD3Q19

For non-specific implementation leave names out. So we do not use generic keyword in the kernel names anymore.
Examples:

  • mus_advRel_kFluid_rBGK_vStd_l
  • mus_advRel_kMsLiquid_rBGK_vStd_l

@haraldkl
Copy link
Member

Here is the new naming convection of the kernels. mus_advRel_k<kind>_r<relaxation>_v<variant>_l<layout> Examples:

* mus_advRel_kFluid_rBGK_v_ld3q19
* mus_advRel_kFluidIncomp_rBGK_ld3q19
* mus_advRel_kFluid_rBGK_vHRR_ld3q19

mus_advRel_kFluidIncomp_rBGK_ld3q19 does not seem to follow the naming convention outlined above (doesn't have a _v part). Is this on purpose to show that some parts might be left out? Or is this a typo?

@KannanMasilamani KannanMasilamani added documentation Improvements or additions to documentation enhancement New feature or request and removed invalid This doesn't seem right labels Apr 18, 2024
@KannanMasilamani
Copy link
Contributor

KannanMasilamani commented Apr 19, 2024

mus_advRel_kFluidIncomp_rBGK_ld3q19 does not seem to follow the naming convention outlined above (doesn't have a _v part). Is this on purpose to show that some parts might be left out? Or is this a typo?

Yes, it was a typo. Corrected it now.

@haraldkl
Copy link
Member

haraldkl commented Jun 5, 2024

@geriJana I believe we can close this issue with the merge of #14. Do you open points from this issue that still need to be addressed?

@geriJana
Copy link
Author

geriJana commented Jun 19, 2024

Thanks! We can close this issue now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants