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 test interdependencies #294

Merged
merged 7 commits into from
Oct 22, 2024
Merged

Fix test interdependencies #294

merged 7 commits into from
Oct 22, 2024

Conversation

jbreue16
Copy link
Contributor

@jbreue16 jbreue16 commented Oct 3, 2024

We found that some tests fail when they are executed in a specific order.

In the course of our investigation, we fixed multiple issues:

  • Fix RowIterator from attempting out-of-bounds access via safeguard
  • Fix memory leaks from multiple allocation (multiple calls of configuration functions)
  • Fix setting the number of AD directions in tests, which caused the issues with interdependencies of tests
  • Further minor issues, see commit log
  • Run benchmark to verify theres no performance decrease from the changes to the rowiterator, see here

Fixes #273

@jbreue16 jbreue16 self-assigned this Oct 3, 2024
@jbreue16 jbreue16 force-pushed the fix/eigenRowIterator branch 2 times, most recently from 7416bed to 54b49d9 Compare October 18, 2024 13:10
@jbreue16 jbreue16 added the bug label Oct 18, 2024
@jbreue16 jbreue16 added this to the v5.0.0 "CADET-Core" milestone Oct 18, 2024
In some loops, the RowIterator was moved beyond the matrix memory,
causing memory issues even if the RowIterator was not used further.
We out-of-bound safeguards are added to prevent this. A benchmark showed
no impact on performance.
@jbreue16 jbreue16 force-pushed the fix/eigenRowIterator branch 2 times, most recently from 3a48754 to 61ae8b1 Compare October 22, 2024 15:55
jbreue16 and others added 3 commits October 22, 2024 18:01
The configure and configureModelDiscretization functions are called
multiple times, which caused multiple allocations for some members of
the unit-operations. This lead to memory leaks, which was especially
severe for the DG code (e.g. 2.5 MB for a single test simulation with
moderate/low discretization) since more members are allocated. This is
fixed by checking if the unit-operation members are already allocated.
For the tests, some units need to initialize the AD directions with
zero. This is not done sometimes when multiple tests are run: The number
of AD directions used might still be set from the previous test, leading
to uninitialized value errors.
Thus, we set the AD directions before configuring the units.

Co-authored-by: Jan Breuer <[email protected]>
@jbreue16 jbreue16 changed the title Fix RowIterator from attempting out-of-bounds access via safeguard Fix test interdependencies Oct 22, 2024
@jbreue16 jbreue16 merged commit 302b487 into master Oct 22, 2024
4 checks passed
@jbreue16 jbreue16 deleted the fix/eigenRowIterator branch October 22, 2024 17:49
@github-actions github-actions bot locked and limited conversation to collaborators Oct 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Non-reproducable CI results
2 participants