Skip to content

Fix off by one error#3406

Merged
ZedThree merged 2 commits into
nextfrom
fix-fci-boundaries
Jun 24, 2026
Merged

Fix off by one error#3406
ZedThree merged 2 commits into
nextfrom
fix-fci-boundaries

Conversation

@dschwoerer

Copy link
Copy Markdown
Contributor

Clearly we need more tests for the FCI boundary operator.

@dschwoerer

Copy link
Copy Markdown
Contributor Author

Cherry-picked from #3330 - which has a test that failed without this fix, as no boundaries where applied.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Ind3D _ind() const { return region->bndry_points[pos].index; }
signed char _boundary_width() const {
return region->localmesh->ystart - region->bndry_points[pos].abs_offset;
return region->localmesh->ystart - region->bndry_points[pos].abs_offset + 1;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: narrowing conversion from 'int' to signed type 'signed char' is implementation-defined [bugprone-narrowing-conversions]

{
             ^

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The signed char as a return type seems unnecessary, but we should at least have static_cast here

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.

👍 Fair point, we later return anyway an int.

@dschwoerer dschwoerer requested a review from ZedThree June 24, 2026 10:07
@ZedThree ZedThree merged commit 4466219 into next Jun 24, 2026
23 checks passed
@ZedThree ZedThree deleted the fix-fci-boundaries branch June 24, 2026 10:14
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.

2 participants