Python like Range#2006
Conversation
|
Hi @llnl/raja-core I've pushed up the discussed changes. |
Co-authored-by: Jason Burmark <MrBurmark@users.noreply.github.com>
Yes absolutely! We are always stronger when we combine our ideas! @rhornung67, @MrBurmark could this be something we carve out time to discuss at the next RAJA meeting? |
Here is a basic prototype without support for things like strongly typed indices. https://godbolt.org/z/o4zdcEEdT |
Regarding making a range where the member variables can be runtime or compile time, that could involve changing the underlying TypedRange class and combining it with the strided version. Maybe we could even unify our integral range types into a single class. Potentially usable in the subviews PR as well #1915. Let's think about doing that as a separate PR. |
|
I pushed up code that aims to address the comments, my question now though if we are happy with the following: Is this too aggressive or should we allow mixing if the underlying data type is the same? I don't use strongly typed indices so I would be interested in how folks would like to handle these cases |
I think these cases should fail, rejecting incorrect index types is the whole point. |
|
Actually thinking about how types are decided, I'm not sure that it makes sense to take the type if the stride into account. If |
right, maybe we can bring it up at the next @llnl/raja-core meeting |
RAJA::range<int>(RangeStrongIndex(3), 17) // is rejected.
RAJA::range<RangeStrongIndex>(AnotherRangeStrongIndex(3), 17) // is rejected.
RAJA::range<RangeStrongIndex>(RangeStrongIndex(3), RangeStrongIndex(17)) // is ok.
RAJA::range<RangeStrongIndex>(RangeStrongIndex(3), 17) // is ok.
RAJA::range<RangeStrongIndex>(3, 17) // is ok.
RAJA::range(RangeStrongIndex(3), RangeStrongIndex(17)) // is ok, chooses RangeStrongIndex.
RAJA::range(RangeStrongIndex(3), 17) // is ok, chooses RangeStrongIndex.
RAJA::range(3, 17) // is ok, chooses int.
RAJA::range(RangeStrongIndex(3), static_cast<long>(17)) // is rejected.
RAJA::range(static_cast<short>(3), 17) // is ok, chooses int.
RAJA::range(3, static_cast<short>(17)) // is ok, chooses int. |
I'm in favor of supporting those that read "ok" -- let me know if there are objections. The only concern is type narrowing, but I think we should build with |
|
I added warnings as errors to CI improvements issue here: #1943 Please add specific items to that list and we will eventually work them in. |
RAJA::range<int>(3, 17, 1) // is ok
RAJA::range<int>(3, 17, static_cast<short>(1)) // is ok
RAJA::range<int>(3, 17, static_cast<long>(1)) // is rejected? or is ok?
RAJA::range<int>(3, 17, RangeStrongIndex(1)) // is rejected
RAJA::range<RangeStrongIndex>(3, 17, 1) // is ok.
RAJA::range<RangeStrongIndex>(RangeStrongIndex(3), RangeStrongIndex(17), 1) // is ok.
RAJA::range<RangeStrongIndex>(RangeStrongIndex(3), RangeStrongIndex(17), RangeStrongIndex(1)) // is ok.
RAJA::range(3, 17, 1) // is ok, chooses int.
RAJA::range(3, 17, static_cast<short>(1)) // is ok, chooses int.
RAJA::range(3, 17, static_cast<long>(1)) // is rejected? or is ok, chooses int?
RAJA::range(3, 17, RangeStrongIndex(1)) // is rejected
RAJA::range(RangeStrongIndex(3), RangeStrongIndex(17), 1) // is ok, chooses RangeStrongIndex.
RAJA::range(RangeStrongIndex(3), 17, 1) // is ok, chooses RangeStrongIndex. |
this would be type narrowing which I think should throw an error |
I've mostly been just lurking here since it looks like you've hit everything I expected @artv3, also I love seeing this idea happen great stuff, but this surprises me. Is it not using |
oh thats right that would happen, but then is that we we want? to promote (3,17) to longs? |
|
Speaking just for me it's what I would want. As long as all three are convertible, which I think common_type requires anyway, it makes it much easier to be sure it's correct. I could easily see someone using the range(1, groups::size) construction as self documenting range and if the upper is an int64_t or unsigned or whatever then it just does the right thing. If they aren't losslessly convertible, like the strongly typed indices, then failing makes sense though. |
|
@trws We do use common type across all of the arguments to get a type for the range if you don't specify one. My question for |
|
Honestly I think it does, at least in as much as when for (std::common_type_t<Begin,End,Stride> iter = begin; iter < end; iter += stride) |
My latest push should support this matrix |
#Summary
Many of my loops start at zero, this gives users a shortcut to generate the desired range segment.