Changing computation at inflow/outflow for tensor solve #1235
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…to inflow_tensor
…to inflow_tensor
…to inflow_tensor
WeiqunZhang
requested changes
Aug 8, 2020
| Array2D<BoundCond,0,2*AMREX_SPACEDIM-1,0,AMREX_SPACEDIM-1> bct; | ||
| for (OrientationIter face; face; ++face) { | ||
| Orientation ori = face(); | ||
| for (int icomp = 0; icomp < AMREX_SPACEDIM; ++icomp) { |
There was a problem hiding this comment.
It will be slightly more efficient to do
for (int icomp ...)
for (OrientationIter ...)
Could you switch the looping order here at similar other places?
There was a problem hiding this comment.
Thanks for the review!! I did the changes as suggested.
WeiqunZhang
approved these changes
Aug 10, 2020
5 tasks
WeiqunZhang
pushed a commit
that referenced
this pull request
Aug 11, 2020
## Summary Correcting tensor cross terms computation for periodic bcs ## Additional background Fixing a bug introduced in PR #1235. Now it computes in the correct way the cross terms in the tensor solve for problems with periodic bcs. ## Checklist The proposed changes: - [x] fix a bug or incorrect behavior in AMReX - [ ] add new capabilities to AMReX - [x] changes answers in the test suite to more than roundoff level - [x] are likely to significantly affect the results of downstream AMReX users - [ ] are described in the proposed changes to the AMReX documentation, if appropriate
WeiqunZhang
added a commit
to WeiqunZhang/amrex
that referenced
this pull request
Aug 13, 2020
ParmParse parameter eb2.extend_domain_face is now true by default. The reason for the change is that for most applications it's hard to imagine one would want it to be false. It will also solve the EB tensor solver issue when there is a cut cell just outside the domain abutting a covered valid cell. Recent changes to the tensor solvers are incorrect and are reverted here. They are incorrect because the ghost cells in the modified functions actually have values at the cell centered. Although the users put domain face values in the ghost cells, that is not what the solver does. The solver stores the boundary values in boundary registers. Before the stencil is being applied, bc function is called to fill the ghost cells with properly interpolated or extrapolated values so that the stencil operations are just like working on normal interior cells. Revert "Correcting tensor cross terms computation for periodic bcs (AMReX-Codes#1254)" This reverts commit 1197adc. Revert "Changing computation at inflow/outflow for tensor solve (AMReX-Codes#1235)" This reverts commit b391885. Revert "Changing computation at outflow for the EBTensor (AMReX-Codes#1187)" This reverts commit 9cd5388.
5 tasks
WeiqunZhang
added a commit
that referenced
this pull request
Aug 14, 2020
## Summary ParmParse parameter eb2.extend_domain_face is now true by default. The reason for the change is that for most applications it's hard to imagine one would want it to be false. It will also solve the EB tensor solver issue when there is a cut cell just outside the domain abutting a covered valid cell. Recent changes to the tensor solvers are incorrect and are reverted here. They are incorrect because the ghost cells in the modified functions actually have values at the cell centered. Although the users put domain face values in the ghost cells, that is not what the solver does. The solver stores the boundary values in boundary registers. Before the stencil is being applied, bc function is called to fill the ghost cells with properly interpolated or extrapolated values so that the stencil operations are just like working on normal interior cells. Revert "Correcting tensor cross terms computation for periodic bcs (#1254)" This reverts commit 1197adc. Revert "Changing computation at inflow/outflow for tensor solve (#1235)" This reverts commit b391885. Revert "Changing computation at outflow for the EBTensor (#1187)" This reverts commit 9cd5388. ## Checklist The proposed changes: - [x] fix a bug or incorrect behavior in AMReX - [ ] add new capabilities to AMReX - [x] changes answers in the test suite to more than roundoff level - [x] are likely to significantly affect the results of downstream AMReX users - [ ] are described in the proposed changes to the AMReX documentation, if appropriate
dwillcox
pushed a commit
to dwillcox/amrex
that referenced
this pull request
Oct 3, 2020
…1235) ## Summary Computation at inflow boundaries for the MLEBTensor and inflow/outflow for MLTensor has been changed. ## Additional background At outflow the tensor solve uses extrapolated values from the interior cell to the outflow face. At inflow, the tensor solve uses boundary data, evaluated at the face, that has been stored in the ghost cell next to the inflow face. ## Checklist The proposed changes: - [x] fix a bug or incorrect behavior in AMReX - [ ] add new capabilities to AMReX - [x] changes answers in the test suite to more than roundoff level - [x] are likely to significantly affect the results of downstream AMReX users - [ ] are described in the proposed changes to the AMReX documentation, if appropriate
dwillcox
pushed a commit
to dwillcox/amrex
that referenced
this pull request
Oct 3, 2020
…es#1254) ## Summary Correcting tensor cross terms computation for periodic bcs ## Additional background Fixing a bug introduced in PR AMReX-Codes#1235. Now it computes in the correct way the cross terms in the tensor solve for problems with periodic bcs. ## Checklist The proposed changes: - [x] fix a bug or incorrect behavior in AMReX - [ ] add new capabilities to AMReX - [x] changes answers in the test suite to more than roundoff level - [x] are likely to significantly affect the results of downstream AMReX users - [ ] are described in the proposed changes to the AMReX documentation, if appropriate
dwillcox
pushed a commit
to dwillcox/amrex
that referenced
this pull request
Oct 3, 2020
## Summary ParmParse parameter eb2.extend_domain_face is now true by default. The reason for the change is that for most applications it's hard to imagine one would want it to be false. It will also solve the EB tensor solver issue when there is a cut cell just outside the domain abutting a covered valid cell. Recent changes to the tensor solvers are incorrect and are reverted here. They are incorrect because the ghost cells in the modified functions actually have values at the cell centered. Although the users put domain face values in the ghost cells, that is not what the solver does. The solver stores the boundary values in boundary registers. Before the stencil is being applied, bc function is called to fill the ghost cells with properly interpolated or extrapolated values so that the stencil operations are just like working on normal interior cells. Revert "Correcting tensor cross terms computation for periodic bcs (AMReX-Codes#1254)" This reverts commit 1197adc. Revert "Changing computation at inflow/outflow for tensor solve (AMReX-Codes#1235)" This reverts commit b391885. Revert "Changing computation at outflow for the EBTensor (AMReX-Codes#1187)" This reverts commit 9cd5388. ## Checklist The proposed changes: - [x] fix a bug or incorrect behavior in AMReX - [ ] add new capabilities to AMReX - [x] changes answers in the test suite to more than roundoff level - [x] are likely to significantly affect the results of downstream AMReX users - [ ] are described in the proposed changes to the AMReX documentation, if appropriate
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Computation at inflow boundaries for the MLEBTensor and inflow/outflow for MLTensor has been changed.
Additional background
At outflow the tensor solve uses extrapolated values from the interior cell to the outflow face. At inflow, the tensor solve uses boundary data, evaluated at the face, that has been stored in the ghost cell next to the inflow face.
Checklist
The proposed changes: