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

Static type checks for Jet::Tensor data #4

Merged
merged 10 commits into from
May 14, 2021
Merged

Conversation

brownj85
Copy link
Collaborator

@brownj85 brownj85 commented May 13, 2021

Context:
Users do not get an error message at compile-time if they create a Tensor with an unsupported data type (i.e. not complex<float> or complex<double>). they will instead get a runtime error when JET_ABORT is called one of the gemm bindings.

Description of the Change:
Adds an is_supported_data_type type trait template to TensorHelpers, in the same vein as std::is_arithmetic, that has a true value if the template parameter is a supported type.

Adds template parameter to disable MultiplyTensorData for unsupported types
Removes JET_ABORT calls from gemm bindings, since they are unreachable.

Adds a static_assert to Jet::Tensor that checks if the templated data type
is supported.

Benefits:
It is now impossible for a user to compile a program using an invalid tensor data type, and there is a clear error message when they attempt to do so.

Possible Drawbacks:
None

Related GitHub Issues:

@brownj85 brownj85 requested review from Mandrenkov and mlxd May 13, 2021 18:49
@github-actions
Copy link

github-actions bot commented May 13, 2021

Test Report (Ubuntu)

    1 files  ±0      1 suites  ±0   0s ⏱️ ±0s
215 tests ±0  215 ✔️ ±0  0 💤 ±0  0 ❌ ±0 
483 runs  ±0  483 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit b9eeabc. ± Comparison against base commit b9eeabc.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented May 13, 2021

Test Report (MacOS)

    1 files  ±0      1 suites  ±0   0s ⏱️ ±0s
215 tests ±0  215 ✔️ ±0  0 💤 ±0  0 ❌ ±0 
483 runs  ±0  483 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit b9eeabc. ± Comparison against base commit b9eeabc.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@Mandrenkov Mandrenkov left a comment

Choose a reason for hiding this comment

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

Awesome work! It's not too often you can improve performance and developer productivity at the same time.

include/jet/TensorHelpers.hpp Outdated Show resolved Hide resolved
.github/CHANGELOG.md Outdated Show resolved Hide resolved
.github/CHANGELOG.md Outdated Show resolved Hide resolved
@Mandrenkov Mandrenkov added the enhancement ✨ New feature or request label May 13, 2021
Copy link
Member

@mlxd mlxd left a comment

Choose a reason for hiding this comment

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

Excellent idea!

Copy link
Collaborator

@Mandrenkov Mandrenkov 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 to me!

@brownj85 brownj85 merged commit b9eeabc into main May 14, 2021
@brownj85 brownj85 deleted the gemmbinding-static-assert branch May 14, 2021 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ✨ New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants