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

make test_lapack more concise #211

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Conversation

germasch
Copy link
Contributor

We do have a bunch of features that help work with matrices / vectors at a time, so let's use more of them. Still not really
as simple as I'd like things to be, but I think it's an improvement.

expect_complex_near(h_B(2, 1, 1), 31.0);
auto expected_B = gt::gtensor<T, 3>(
{{{1., 2., 3.}, {-3., 7., 31.}}, {{1., 2., 3.}, {-3., 7., 31.}}});
EXPECT_NEAR(gt::sum_squares(h_B - expected_B), 0., 1e-10);
Copy link
Contributor

Choose a reason for hiding this comment

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

The expect_complex_near helper can show which elements triggered the failure. This is good for small matrices, and awful for large ones, so my preference would be to keep the helper for this small case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh but this is not using the array helper but the individual one... this one is good for small arrays: https://github.com/wdmapp/gtensor/blob/main/tests/test_helpers.h#L110

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, you had another helper somewhere which flattened the two gtensors and then compared element by element -- that would be a nice compromise, since it's also a concise way of expressing stuff. I guess overall this kinda points out that we don't have proper iterators for gtensors, the best we can do is flatten and do a 1-d loop. Though now that I think about it, I guess we could use gt::launch with EXPECT inside of it, that'd seem neat enough.

@germasch
Copy link
Contributor Author

Learned two things from the looking at the sycl CI:

  • We're still testing a function that I declared deprecated.
  • Apparently sycl doesn't like the ∂ symbol in the code, which gcc/clang didn't seem to mind. Of course they shouldn't have been in there in the first place..

They are still kinda lengthy compared to what they do -- we should
probably have a better way to deal with the host/device mirroring.

And generally, there's still more room for improvement, I don't think
I made all the best choices here...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants