-
Notifications
You must be signed in to change notification settings - Fork 91
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
Rename half helper #1736
Rename half helper #1736
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these are the only instances I could find where the 'normal' version can be used instead if the '*Base' version.
I can't go through every file by itself, so I just searched where the '*Base' version still appear.
@@ -34,7 +34,7 @@ class Csr : public CommonTestFixture { | |||
protected: | |||
using Arr = gko::array<int>; | |||
using Vec = gko::matrix::Dense<value_type>; | |||
using Vec2 = gko::matrix::Dense<gko::next_precision<value_type>>; | |||
using Vec2 = gko::matrix::Dense<gko::next_precision_base<value_type>>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using Vec2 = gko::matrix::Dense<gko::next_precision_base<value_type>>; | |
using Vec2 = gko::matrix::Dense<gko::next_precision<value_type>>; |
Although I think this is not used at all. (But that is nothing to change in this PR.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still think this should be changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is the same reason for keeping that. the value_type will be float with enabling dpcpp_single_mode.
I would rather deleting them because they are not used.
27d04f7
to
184a2b9
Compare
@@ -34,7 +34,7 @@ class Csr : public CommonTestFixture { | |||
protected: | |||
using Arr = gko::array<int>; | |||
using Vec = gko::matrix::Dense<value_type>; | |||
using Vec2 = gko::matrix::Dense<gko::next_precision<value_type>>; | |||
using Vec2 = gko::matrix::Dense<gko::next_precision_base<value_type>>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still think this should be changed.
This PR renames the temporary naming scheme for half.
next_precision
->next_precision_base
(only float and double)next_precision_with_half
->next_precision
(It performs the same as the previous release ifGINKGO_ENABLE_HALF
isOFF
)GKO_INSTANTIATE_FOR_EACH_VALUE_TYPE
->GKO_INSTANTIATE_FOR_EACH_VALUE_TYPE_BASE
GKO_INSTANTIATE_FOR_EACH_VALUE_TYPE_WITH_HALF
->GKO_INSTANTIATE_FOR_EACH_VALUE_TYPE
ValueTypes
->ValueTypesBase
ValueTypesWithHalf
->ValueTypes
value_type_list()
->value_type_list_base()
value_type_list_with_half()
->value_type_list()
We might go for the same way as what we did in the dpcpp_single_mode, which keeps the interface but throw the error for double precision implementation in the future. From the public interface perspective, we can go from the current status to there without hurting, but not good via the other direction. Thus, we keep the original plan for this release now.
The following script (not simple rule though)
using it with
sed -E
the last commit contains tiny manual change to turn some unnecessary change as they are helper without type information actually