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

Clear some forgotten fields in Triangulation::clear(). #15877

Merged
merged 2 commits into from Aug 17, 2023

Conversation

bangerth
Copy link
Member

In 1d, triangulations store maps from vertex index to manifold and boundary ids. We forgot to clear these fields in Triangulation::clear() (or rather the internal function clear() calls).

I don't think this has an effect that is observable because once you create another triangulation in the object, we add or overwrite the elements of this map, and when we access it we only ever access the elements we have just added or overwritten -- elements that just happen to still be there are simply never accessed, but they take up memory and we should really reset these maps in clear() as well.

I found this in #15689 where I actually am accessing all elements of these maps in order to translate vertex numbers, and I'm running into cases where some of the elements are invalid (because left-over from a triangulation we no longer store). This patch fixes this.

Copy link
Member

@tjhei tjhei left a comment

Choose a reason for hiding this comment

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

Can you use .reset() instead? Afterwards, the comment will be redundant too.

@bangerth
Copy link
Member Author

.reset() resets the pointer to a nullptr. What I need is to let the pointer point to an empty object.

Or are you suggesting to call .reset() on the pointed-to object? Since the object pointed to is a map, you would then mean to call .clear() on it?

@tjhei
Copy link
Member

tjhei commented Aug 17, 2023

Sorry, you are right. Are they guaranteed to be non null? If yes, .clear () seems like a sensible thing to do or do you disagree?

@bangerth
Copy link
Member Author

Yes, they are guaranteed to be non-null -- but one can also assert this :-)

I've pushed another patch that makes the modification you suggest (and that adds more assertions as well).

@marcfehling marcfehling merged commit 7a82275 into dealii:master Aug 17, 2023
15 checks passed
@bangerth bangerth deleted the fix-clear branch August 17, 2023 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants