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.

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

Small Bugfix: Remove possibility of bitshifting negative integers #1111

Merged
merged 6 commits into from
Jun 18, 2024
Merged

Small Bugfix: Remove possibility of bitshifting negative integers #1111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
be633cd
remove possible bitshifts of negative numbers
lroberts36 Jun 13, 2024
83cf308
changelog
lroberts36 Jun 13, 2024
cd6f2be
Merge branch 'develop' into lroberts36/fix-bitshift-of-negative
lroberts36 Jun 15, 2024
0ddf5cd
Merge branch 'develop' into lroberts36/fix-bitshift-of-negative
lroberts36 Jun 15, 2024
6a947a9
Merge branch 'develop' into lroberts36/fix-bitshift-of-negative
BenWibking Jun 17, 2024
a48108d
Merge branch 'develop' into lroberts36/fix-bitshift-of-negative
pgrete Jun 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
- [[PR 1004]](https://github.com/parthenon-hpc-lab/parthenon/pull/1004) Allow parameter modification from an input file for restarts

### Fixed (not changing behavior/API/variables/...)
- [[PR 1111]](https://github.com/parthenon-hpc-lab/parthenon/pull/1111) Fix undefined behavior due to bitshift of negative number in LogicalLocation
- [[PR 1092]](https://github.com/parthenon-hpc-lab/parthenon/pull/1092) Updates to DataCollection and MeshData to remove requirement of predefining MeshBlockData
- [[PR 1113]](https://github.com/parthenon-hpc-lab/parthenon/pull/1113) Prevent division by zero
- [[PR 1112]](https://github.com/parthenon-hpc-lab/parthenon/pull/1112) Remove shared_ptr cycle in forest::Tree
Expand Down
8 changes: 4 additions & 4 deletions src/mesh/forest/logical_location.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,10 @@ bool LogicalLocation::IsNeighbor(const LogicalLocation &in) const {

bool neighbors = true;
for (int dir = 0; dir < 3; ++dir) {
auto low = (l(dir) << level_shift_this) - 1;
auto low = l(dir) * block_size_this - 1;
auto hi = low + block_size_this + 1;

auto low_in = (in.l(dir) << level_shift_in);
auto low_in = in.l(dir) * block_size_in;
auto hi_in = low_in + block_size_in - 1;
Comment on lines -121 to 125
Copy link
Collaborator

Choose a reason for hiding this comment

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

Out of curiosity, where does the asymmetry come from (i.e., that low/hi is offset by-1/+1 and low_in, hi_in by 0,-1)?
Would be great to also add a short comment in the code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It just has to do with how the overlap is checked, i.e. the bounds of this are increased by one in the positive and negative direction to create a one layer thick ghost region.

neighbors = neighbors && !(hi < low_in || low > hi_in);
}
Expand All @@ -140,7 +140,7 @@ bool LogicalLocation::IsNeighborOfTE(const LogicalLocation &in,

bool neighbors = true;
for (int dir = 0; dir < 3; ++dir) {
auto low = (l(dir) << level_shift_this);
auto low = l(dir) * block_size_this;
auto hi = low + block_size_this - 1;
if (te_offset[dir] == -1) {
low -= 1;
Expand All @@ -150,7 +150,7 @@ bool LogicalLocation::IsNeighborOfTE(const LogicalLocation &in,
low = hi - 1;
}

auto low_in = (in.l(dir) << level_shift_in);
auto low_in = in.l(dir) * block_size_in;
auto hi_in = low_in + block_size_in - 1;
neighbors = neighbors && !(hi < low_in || low > hi_in);
}
Expand Down
Loading