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.

Sign up for GitHub

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

Jump to bottom

Boundary condition contribution to gradient system #198

Open

trevilo opened this issue Feb 21, 2023 · 0 comments

Assignees

Labels

bug

Contributor

trevilo commented Feb 21, 2023

There is a significant bug in the face integration term on the right-hand-side of the gradient system for the BR1 implementation. Specifically, it neglects the contribution of the boundary faces, which implicitly assumes that the interior state satisfies the boundary conditions.

trevilo added the bug label

trevilo self-assigned this

trevilo added a commit that referenced this issue


          Eliminate some dead _GPU_ code.

4e28e4c

Specifically, parts of GradNonLinearForm and GradFaceIntegrator had
_GPU_ specific code that was superseded by device-specific code in the
Gradients class and thus was never called.  This code has been
eliminated to improve hygiene in preparation for changes to address #198.

trevilo added a commit that referenced this issue


          Some clean up in class GradFaceIntegrator

04a8ced

In preparation for further refactoring as part of addressing #198

trevilo added a commit that referenced this issue


          Some logic to be able to use BC info in GradFaceIntegrator (#198)

7091c6a

The idea here is to take advantage of all the data that has already
been set up in class BCIntegrator by making class GradFaceIntegrator
its friend.  Then we can use the internal std::unordered_map objects
in BCIntegrator, which map the boundary attribute to the appropriate
BoundaryCondition class, to get access to any required boundary
condition information.  In principle that is all that is necessary,
but the data we need it is not in a convenient form.  To address this,
the method computeBdrPrimitiveStateForGradient has been added to the
BoundaryCondition class.  In the base class, this method just sets the
boundary state to the internal state, which results in the same
(incorrect) behavior we have now.  In future commits, this method will
be implemented in the children of BoundaryCondition in order to
provide the right boundary state.

trevilo added a commit that referenced this issue


          Add option specifying whether to include BC terms in gradient calc (#198

ec0449e

)

Eventually useBCinGrad = true will be the default (and only) option.
But, during development, it is useful to be able to turn it off so we
can ensure we don't introduce any unintended effects by running the
regression tests (which will have to have their 'truth' solutions
regenerated once this change is ready.

trevilo added a commit that referenced this issue


          Towards #198 --- Add primitive BC for isothermal wall

7ae0384

trevilo added a commit that referenced this issue


          Eliminate some dead _GPU_ code.

6867a3f

Specifically, parts of GradNonLinearForm and GradFaceIntegrator had
_GPU_ specific code that was superseded by device-specific code in the
Gradients class and thus was never called.  This code has been
eliminated to improve hygiene in preparation for changes to address #198.

trevilo added a commit that referenced this issue


          Some clean up in class GradFaceIntegrator

5fc8345

In preparation for further refactoring as part of addressing #198

trevilo added a commit that referenced this issue


          Some logic to be able to use BC info in GradFaceIntegrator (#198)

ae6fdda

The idea here is to take advantage of all the data that has already
been set up in class BCIntegrator by making class GradFaceIntegrator
its friend.  Then we can use the internal std::unordered_map objects
in BCIntegrator, which map the boundary attribute to the appropriate
BoundaryCondition class, to get access to any required boundary
condition information.  In principle that is all that is necessary,
but the data we need it is not in a convenient form.  To address this,
the method computeBdrPrimitiveStateForGradient has been added to the
BoundaryCondition class.  In the base class, this method just sets the
boundary state to the internal state, which results in the same
(incorrect) behavior we have now.  In future commits, this method will
be implemented in the children of BoundaryCondition in order to
provide the right boundary state.

trevilo added a commit that referenced this issue


          Add option specifying whether to include BC terms in gradient calc (#198

cbd9aa0

)

Eventually useBCinGrad = true will be the default (and only) option.
But, during development, it is useful to be able to turn it off so we
can ensure we don't introduce any unintended effects by running the
regression tests (which will have to have their 'truth' solutions
regenerated once this change is ready.

trevilo added a commit that referenced this issue


          Towards #198 --- Add primitive BC for isothermal wall

92d09b5

trevilo added a commit that referenced this issue


          Eliminate some dead _GPU_ code.

cec7f03

Specifically, parts of GradNonLinearForm and GradFaceIntegrator had
_GPU_ specific code that was superseded by device-specific code in the
Gradients class and thus was never called.  This code has been
eliminated to improve hygiene in preparation for changes to address #198.

trevilo added a commit that referenced this issue


          Some clean up in class GradFaceIntegrator

e995129

In preparation for further refactoring as part of addressing #198

trevilo added a commit that referenced this issue


          Some logic to be able to use BC info in GradFaceIntegrator (#198)

10fa769

The idea here is to take advantage of all the data that has already
been set up in class BCIntegrator by making class GradFaceIntegrator
its friend.  Then we can use the internal std::unordered_map objects
in BCIntegrator, which map the boundary attribute to the appropriate
BoundaryCondition class, to get access to any required boundary
condition information.  In principle that is all that is necessary,
but the data we need it is not in a convenient form.  To address this,
the method computeBdrPrimitiveStateForGradient has been added to the
BoundaryCondition class.  In the base class, this method just sets the
boundary state to the internal state, which results in the same
(incorrect) behavior we have now.  In future commits, this method will
be implemented in the children of BoundaryCondition in order to
provide the right boundary state.

trevilo added a commit that referenced this issue


          Add option specifying whether to include BC terms in gradient calc (#198

1cfdbd0

)

Eventually useBCinGrad = true will be the default (and only) option.
But, during development, it is useful to be able to turn it off so we
can ensure we don't introduce any unintended effects by running the
regression tests (which will have to have their 'truth' solutions
regenerated once this change is ready.

trevilo added a commit that referenced this issue


          Towards #198 --- Add primitive BC for isothermal wall

e16ee49

trevilo added a commit that referenced this issue


          baby steps on infrastructure to get grad BC fix into gpu path (#198)

255899e

trevilo added a commit that referenced this issue


          Eliminate some dead _GPU_ code.

Specifically, parts of GradNonLinearForm and GradFaceIntegrator had
_GPU_ specific code that was superseded by device-specific code in the
Gradients class and thus was never called.  This code has been
eliminated to improve hygiene in preparation for changes to address #198.

trevilo added a commit that referenced this issue


          Some clean up in class GradFaceIntegrator

cb5455e

In preparation for further refactoring as part of addressing #198

trevilo added a commit that referenced this issue


          Some logic to be able to use BC info in GradFaceIntegrator (#198)

6b68c92

The idea here is to take advantage of all the data that has already
been set up in class BCIntegrator by making class GradFaceIntegrator
its friend.  Then we can use the internal std::unordered_map objects
in BCIntegrator, which map the boundary attribute to the appropriate
BoundaryCondition class, to get access to any required boundary
condition information.  In principle that is all that is necessary,
but the data we need it is not in a convenient form.  To address this,
the method computeBdrPrimitiveStateForGradient has been added to the
BoundaryCondition class.  In the base class, this method just sets the
boundary state to the internal state, which results in the same
(incorrect) behavior we have now.  In future commits, this method will
be implemented in the children of BoundaryCondition in order to
provide the right boundary state.

trevilo added a commit that referenced this issue


          Add option specifying whether to include BC terms in gradient calc (#198

d9a1949

)

Eventually useBCinGrad = true will be the default (and only) option.
But, during development, it is useful to be able to turn it off so we
can ensure we don't introduce any unintended effects by running the
regression tests (which will have to have their 'truth' solutions
regenerated once this change is ready.

trevilo added a commit that referenced this issue


          Towards #198 --- Add primitive BC for isothermal wall

6d68dd8

trevilo added a commit that referenced this issue


          baby steps on infrastructure to get grad BC fix into gpu path (#198)

bc30634

trevilo added a commit that referenced this issue


          Eliminate some dead _GPU_ code.

a720e33

Specifically, parts of GradNonLinearForm and GradFaceIntegrator had
_GPU_ specific code that was superseded by device-specific code in the
Gradients class and thus was never called.  This code has been
eliminated to improve hygiene in preparation for changes to address #198.

trevilo added a commit that referenced this issue


          Some clean up in class GradFaceIntegrator

032c82a

In preparation for further refactoring as part of addressing #198

trevilo added a commit that referenced this issue


          Some logic to be able to use BC info in GradFaceIntegrator (#198)

0acbc69

The idea here is to take advantage of all the data that has already
been set up in class BCIntegrator by making class GradFaceIntegrator
its friend.  Then we can use the internal std::unordered_map objects
in BCIntegrator, which map the boundary attribute to the appropriate
BoundaryCondition class, to get access to any required boundary
condition information.  In principle that is all that is necessary,
but the data we need it is not in a convenient form.  To address this,
the method computeBdrPrimitiveStateForGradient has been added to the
BoundaryCondition class.  In the base class, this method just sets the
boundary state to the internal state, which results in the same
(incorrect) behavior we have now.  In future commits, this method will
be implemented in the children of BoundaryCondition in order to
provide the right boundary state.

trevilo added a commit that referenced this issue


          Add option specifying whether to include BC terms in gradient calc (#198

1db8986

)

Eventually useBCinGrad = true will be the default (and only) option.
But, during development, it is useful to be able to turn it off so we
can ensure we don't introduce any unintended effects by running the
regression tests (which will have to have their 'truth' solutions
regenerated once this change is ready.

trevilo added a commit that referenced this issue


          Towards #198 --- Add primitive BC for isothermal wall

fbce851

trevilo mentioned this issue

Toward boundary gradient fix: part 1 #216

Merged

trevilo mentioned this issue

Port mixing length model to the gpu path (and gradient fix, part 2) #226

Merged

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