-
Notifications
You must be signed in to change notification settings - Fork 87
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
Inconsistent use of integer types #430
Comments
Coming back to this, since @thoasm and I discussed some related issues or questions about code style, I want to propose a few changes that might clean up our code base.
Comments? |
I agree, making everything a |
Yes, that would be a long-term goal. Maybe we can collect other changes/issues that this switch would cause. Until then, I'll try to use static_cast where possible |
Changing to For nnz >= MAX_INT, (the num stored elements) |
After working on #464, I now have a slightly clearer picture of what the impact of the proposed changes would be, so my updated proposal would be:
Comments? |
It seems to me Ginkgo is sometimes integer types inconsistently.
This includes both 32/64 bit as well as
signed
/unsigned
. Examples:dim
returns asize_type
for all dimensions, even though we internally use theIndexType
template parameter in almost all kernels.size_type
as loop variables.size_type
My opinion: I would consistently use signed types everywhere, because we use them for indexing anyways and overflows on signed types are way easier to recognize/catch when debugging. Only for allocation sizes and/or launch bounds, it might make sense to use
size_type
. But that is often a very fundamental debate, so I would like to start it here.On the question what type
dim
should use, I am conflicted, since it would probably be annoying having to typedim<IndexType>
everywhere, but in my opinion, it should at least return a signed type.The text was updated successfully, but these errors were encountered: