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

Unorderedmap clear() should zero the size() #4130

Merged
merged 3 commits into from
Jul 14, 2021

Conversation

nliber
Copy link
Contributor

@nliber nliber commented Jun 29, 2021

Fix for issue #4128.

UnorderedMap::m_size is a cache of the size of the container (that is why is is marked mutable), which only gets updated when size() is called on a modified (indicated by the m_scalars(modified_idx) flag) map. This flag is only possibly set in insert and erase methods. clear() both clears out m_scalars (which effectively clears the modified_idx flag) and doesn't update the m_size value, hence the bug.

I did verify that if we always recalculate the size, the correct value of 0 is calculated.

@nliber nliber changed the title Unorderedmap clear() zero size Unorderedmap clear() should zero the size() Jun 29, 2021
@crtrott
Copy link
Member

crtrott commented Jun 30, 2021

Formatting issue.

Copy link
Contributor

@masterleinad masterleinad left a comment

Choose a reason for hiding this comment

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

You still need to fix the indentation.

containers/src/Kokkos_UnorderedMap.hpp Show resolved Hide resolved
Copy link
Contributor

@masterleinad masterleinad left a comment

Choose a reason for hiding this comment

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

Looks good after fixing the indentation.

@crtrott crtrott added this to In progress in Kokkos Release 3.5 Jul 14, 2021
@crtrott crtrott moved this from In progress to Awaiting Feedback in Kokkos Release 3.5 Jul 14, 2021
@crtrott crtrott merged commit 790f903 into kokkos:develop Jul 14, 2021
Kokkos Release 3.5 automation moved this from Awaiting Feedback to Done Jul 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants