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 Memory Leak in LSPG ROM Builder and Solver by Ensuring Vector Zeroing #12342

Merged
merged 4 commits into from
May 6, 2024

Conversation

SADPR
Copy link
Contributor

@SADPR SADPR commented May 3, 2024

Description

This PR resolves an issue with improper initialization found in the LSPG ROM builder and solver test (the bug was in the global_rom_builder_and_solver.h). The problem was due to vectors not being zeroed out after resizing, leading to uninitialized floating point values, which could affect the accuracy and reliability of computations.

@SADPR SADPR self-assigned this May 3, 2024
@matekelemen
Copy link
Contributor

I'm relatively sure that the values within a floating point array are not the root cause of a memory leak. Have you tried running the leaky case with ASAN or Valgrind?

@SADPR
Copy link
Contributor Author

SADPR commented May 6, 2024

I'm relatively sure that the values within a floating point array are not the root cause of a memory leak. Have you tried running the leaky case with ASAN or Valgrind?

You're right, it actually wasn't a memory leak. We found out that the issue was just improper initialization of the vector. We weren't setting the values to zero after resizing, which was causing the problems. We've fixed that now.

@SADPR SADPR marked this pull request as ready for review May 6, 2024 08:23
@SADPR SADPR requested a review from a team as a code owner May 6, 2024 08:23
@SADPR
Copy link
Contributor Author

SADPR commented May 6, 2024

@RiccardoRossi ready!

@SADPR SADPR merged commit 44161e1 into master May 6, 2024
11 checks passed
@SADPR SADPR deleted the Kratos_ROM_TestLSPG branch May 6, 2024 12:26
@roigcarlo
Copy link
Member

Just to answer @matekelemen,

We passed valgrind with leak check full and this was the only problem. I run it with ASAN some weeks ago and there was nothing reported.

Of topic: Valgrind does report a problem while running with openmp with ~1K possible lost per OMP_THREAD, but has nothing to do with this test in particular

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

8 participants