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 uninitialized variable #16750

Merged
merged 4 commits into from Mar 16, 2024

Conversation

kronbichler
Copy link
Member

I saw this uninitialized variable when running a relaxation preconditioner with valgrind, and I believe it will also fix the error reported here:
https://cdash.dealii.org/viewTest.php?onlyfailed&buildid=2516

Related to #16747.

@kronbichler kronbichler added Linear Algebra ready to test Regression tester Issues reported by the regression tester bot labels Mar 15, 2024
@kronbichler
Copy link
Member Author

With this change, the test is now failing in the first three lines,

2,4c2,4
< DEAL::OK!
< DEAL::OK!
< DEAL::OK!
---
> DEAL::ERROR!
> DEAL::ERROR!
> DEAL::ERROR!

This corresponds to the relaxation parameter 0.0, i.e., automatic determination of eigenvalues, running for 1, 2, 3 sweeps. However, the test is designed in such a way that it is really hard to see where exactly we are failing now. @peterrum would you mind looking into the tests as to where they fail? I think this just succeeded by chance before, depending on the spurious value of uninitialized variable. In any case, it would also be desirable if the test could print more precisely where something fails, rather than just accumulate errors in a spaghetti manner over 4 different metrics and 5 different test cases within each variant without printing specific information here:

if (std::all_of(results.begin(), results.end(), [&](const auto &a) {
if (std::abs(std::get<0>(a) - std::get<0>(results[0])) > 1e-6)
return false;
if (std::abs(std::get<1>(a) - std::get<1>(results[0])) > 1e-6)
return false;
if (std::abs(std::get<2>(a) - std::get<2>(results[0])) > 1e-6)
return false;
if (std::abs(std::get<3>(a) - std::get<3>(results[0])) > 1e-6)
return false;
return true;
}))

@peterrum
Copy link
Member

I will do.

@kronbichler
Copy link
Member Author

Thanks for the help!

@peterrum
Copy link
Member

@kronbichler I have fixed the test and added some asserts.

@kronbichler
Copy link
Member Author

Thank you @peterrum, this looks much better now!

DEAL::OK! 12.9800 21.7988 12.9800 21.7988
DEAL::OK! 6.18750 14.8024 6.18750 14.8024
DEAL::OK! 10.7121 19.5161 10.7121 19.5161
DEAL::OK! 15.6708 24.5042 15.6708 24.5042
Copy link
Member

Choose a reason for hiding this comment

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

Let's make this output more than 10 digits. Otherwise, numdiff might trigger accidentally.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea, I adjusted the test and its output.

@kronbichler kronbichler merged commit b9f7503 into dealii:master Mar 16, 2024
16 checks passed
@kronbichler kronbichler deleted the fix_uninitialized_variable branch March 18, 2024 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Linear Algebra ready to test Regression tester Issues reported by the regression tester bot
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants