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

EAMxx: improve horiz/vert contraction tests #6897

Merged
merged 4 commits into from
Jan 17, 2025

Conversation

mahf708
Copy link
Contributor

@mahf708 mahf708 commented Jan 15, 2025

imrpoves contraction testing by using catch2 matchers to allow for tolerance and expand contracted dimensions.

Fixes #6893

@mahf708 mahf708 added enhancement Testing Anything related to unit/system tests EAMxx PRs focused on capabilities for EAMxx labels Jan 15, 2025
@mahf708 mahf708 requested a review from bartgol January 15, 2025 03:39
for(int i = 0; i < dim0; ++i) {
for(int j = 0; j < dim1; ++j) {
for(int k = 0; k < dim2; ++k) {
mr3(i, j) += v1(i, k) * v2(i, j, k);
}
REQUIRE_THAT(rr3(i, j), Catch::Matchers::WithinRel(mr3(i, j), tol));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bartgol I decided to keep the manual unrolling stuff because I thought it is better to keep utils isolated in their testing (kinda with a reference of frame of basics) instead of using more advanced mathy construction

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good. Personally, I find mathy more readable than impl-rich loops with views access. After all, once the utils are tested, why not using them inside other tests?

But really, this is fine, it's just a matter of taste. :)

@mahf708 mahf708 changed the title improve contraction tests EAMxx: improve horiz/vert contraction tests Jan 15, 2025
Copy link
Contributor

@bartgol bartgol 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. I have a couple of comments, but nothing too important.

@mahf708 mahf708 requested a review from bartgol January 16, 2025 23:01
@bartgol bartgol merged commit 1df5466 into master Jan 17, 2025
20 checks passed
@bartgol bartgol deleted the mahf708/eamxx/contraction-tests branch January 17, 2025 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EAMxx PRs focused on capabilities for EAMxx enhancement Testing Anything related to unit/system tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

improve test robustness comparing regarding order-of-ops
2 participants