Skip to content

Minor fix for NekBinnedPlaneIntegral use in non-dimensional cases#882

Open
anshchaube wants to merge 2 commits into
neams-th-coe:develfrom
anshchaube:nbpi_fix
Open

Minor fix for NekBinnedPlaneIntegral use in non-dimensional cases#882
anshchaube wants to merge 2 commits into
neams-th-coe:develfrom
anshchaube:nbpi_fix

Conversation

@anshchaube

Copy link
Copy Markdown
Contributor

Small fix that dimensionalises the _gap_width parameter for use with non-dimensional cases. Without it, the volume integral from the bins gets multiplied by L_ref^3, resulting in incorrect scaling for a userobject that should ultimately yield an area (for an appropriate gap_width).

@moosebuild

Copy link
Copy Markdown
Collaborator

Job Precheck on 9ef4fff wanted to post the following:

Your code requires style changes.

A patch was auto generated and copied here
You can directly apply the patch by running, in the top level of your repository:

curl -s https://mooseframework.inl.gov/cardinal/docs/PRs/882/clang_format/style.patch | git apply -v

Alternatively, with your repository up to date and in the top level of your repository:

git clang-format 0d1f9b2a9c57c9b0d69d67fda9b226266e07bf6e

@moosebuild

Copy link
Copy Markdown
Collaborator

Job Documentation on 8b8e8c4 wanted to post the following:

View the site here

This comment will be updated on new commits.

@anshchaube anshchaube changed the title minor fix for non-dimensional cases Minor fix for NekBinnedPlaneIntegral use in non-dimensional cases May 14, 2024
// correct the values to areas
for (unsigned int i = 0; i < _n_bins; ++i)
_bin_values[i] /= _gap_thickness;
_bin_values[i] /= (_gap_thickness * nekrs::referenceLength());

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't it be divided by, not multiplied? That way, _gap_thickness is always specified in dimensional form (like the rest of the quantities in NekRSProblem).

@anshchaube anshchaube May 15, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have been treating _gap_thickness as non-dimensional when the mesh has been non-dimensionalized. After scaling the mesh in the usr file, I have noticed that treating gap_thickness as dimensional results in errors like:

The following error occurred in the UserObject 'pressure_planar_integral' of type NekBinnedPlaneIntegral.

Failed to map any GLL points to bin 1!

I have had better luck treating it as non-dimensional when the mesh has been non-dimensionalized. However, that means at the end of the process, it needs to be re-dimensionalized so that after dimensionalizeVolume acts on the integral and multiplies it by L_ref^3, the division by L_ref at the end results in a final scaling factor of L_ref^2 for the area. I have verified this by using Nek's internal routines to calculate the same area.

Please let me know if the intended usage is different.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also prefer treating _gap_thickness as non-dimensional when the mesh is non-dimensional because I can get a sense of the gap thickness directly from the Nek output field files.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The convention in Cardinal is that all parameters in the nek.i files are dimensional, though - for instance, if we use a NekPointValue, the point is specified in dimensional form. I am happy to merge this PR, but only if it is consistent with the rest of the accessors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants