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.

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

Fix incorrect assert in Scatterer #2224

Merged
merged 9 commits into from
Jun 10, 2022
Merged

Fix incorrect assert in Scatterer #2224

merged 9 commits into from
Jun 10, 2022

Conversation

jpdean
Copy link
Member

@jpdean jpdean commented Jun 1, 2022

After discussing with @jorgensd, it looks like this assert statement might be incorrect. local_data has size size_local() * bs(), but the indices in _local_inds are grouped by the processes in the neighbourhood, so for some index maps, _local_inds.size() may be larger than local_data.size().

This PR removes the assert statement, but perhaps it could be replaced.

@IgorBaratta
Copy link
Member

You're right, the assert statement is incorrect.
The correct statement would be something like:

assert(*std::max_element(_local_inds.begin(), _local_inds.end()) < std::int32_t(local_data.size()));

@jpdean jpdean changed the title Remove incorrect assert in Scatterer Fix incorrect assert in Scatterer Jun 6, 2022
@jpdean jpdean requested a review from garth-wells June 7, 2022 11:00
@jorgensd jorgensd added the bug Something isn't working label Jun 9, 2022
@garth-wells garth-wells merged commit 21247b6 into main Jun 10, 2022
@garth-wells garth-wells deleted the jpdean/scatterer_fix branch June 10, 2022 06:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants