Convert IntVect to IntVectND #3969
Conversation
## Summary This PR adds a `TupleSplit` function for `GpuTuple` similar to `IntVectSplit` in #3969. It can be used as an inverse function of `TupleCat` and `TypeMultiplier`. Example: ```C++ auto tup = amrex::makeTuple(2,4,5,7,2.324,7,8,342.3f,4ull,1ll,-38,"test"); auto [t0,t1,t2,t3] = amrex::TupleSplit<3,3,4,2>(tup); // t0 = GpuTuple( 2, 4, 5 ) // t1 = GpuTuple( 7, 2.324, 7 ) // t2 = GpuTuple( 8, 342.2999878, 4, 1 ) // t3 = GpuTuple( -38, test ) ``` ## Additional background The current implementation does not move the tuple elements and makes a copy instead.
|
Should we have |
|
Wouldn't that prevent the member functions from getting inlined, which could be bad when IntVect is used in a parallel for? |
|
My understanding is it should not affect run time performance. But I might be wrong. |
|
It is also not clear how GPU compilers treat it. So probably a bad idea and
let's not worry about it.
…On Tue, Jun 11, 2024, 10:05 AM Alexander Sinn ***@***.***> wrote:
Wouldn't that prevent the member functions from getting inlined, which
could be bad when IntVect is used in a parallel for?
—
Reply to this email directly, view it on GitHub
<#3969 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB37TYKFM7RBL3YJMPHJYYTZG4U4VAVCNFSM6AAAAABIUM7VYCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRRGIZTKOBSHE>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
|
I tried to add |
|
Maybe it's because all those functions have already been inlined. |
This seems to work. So we could get IntVect::Zero and IntVect::Unit back for backward compatibility. Unfortunately |
|
Unfortunately, nvcc does not work. |
|
Maybe instead of being IntVectND, Zero and Unit could have a different type that is implicitly convertible to IntVectND. This would allow them to be |
|
That may not work in cases like This is copied from a real code. Maybe we can try to figure out how to make nvcc happy. The error message is |
|
Maybe it wants |
|
It seems to work if both are just const with no constexpr. |
|
Great. Maybe it will even work for all |
|
constexpr const actually works somehow |
|
@ax3l This PR will break pyamrex. How do you want to handle this? Should we wait till pyamrex has a PR ready? |
|
Thanks for the ping. That is all we need to track. Please go ahead here to merge and we update pyAMReX shortly after. |
|
pyAMReX update coming via AMReX-Codes/pyamrex#332 :) |
## Summary Similar to #3969 but for IndexType. ## Additional background A maximum of 31 dimensions are supported so that (1u << dim) can fit into an unsigned int.
## Summary Similar to #3969 and #3988 but for Box. ## Additional background It should be checked that the changes to BoxIndexer do not affect the compiled GPU code. In my testing, it gives the same performance as development. Example usage: ```C++ amrex::BoxND b1{amrex::IntVectND{1,2,3}, amrex::IntVectND{4,5,6}, amrex::IntVectND{1,0,1}}; // ((1,2,3) (4,5,6) (1,0,1)) auto b2 = amrex::BoxCat(b1, b1, b1); // ((1,2,3,1,2,3,1,2,3) (4,5,6,4,5,6,4,5,6) (1,0,1,1,0,1,1,0,1)) auto [b3, b4, b5, b6, b7] = amrex::BoxSplit<1, 4, 2, 1, 1>(b2); // ((1) (4) (1))((2,3,1,2) (5,6,4,5) (0,1,1,0))((3,1) (6,4) (1,1))((2) (5) (0))((3) (6) (1)) auto b8 = amrex::BoxResize<2>(b4); // ((2,3) (5,6) (0,1)) auto b9 = amrex::BoxResize<5>(b8); // ((2,3,0,0,0) (5,6,0,0,0) (0,1,0,0,0)) ```
Summary
As described in #3955, this PR converts IntVect to the n dimensional IntVectND and adds
using IntVect = IntVectND<AMREX_SPACEDIM>;. Additionally, deduction guides, support for structured bindings and the helper functionsIntVectCat,IntVectSplitandIntVectResizewere added to IntVectND.I didn't find a way to make
IntVect::ZeroandIntVect::Unitwork with n dimensions, so I removed them (converting them tostatic constexprorinline staticdoesn't work with MSVC). Instead,IntVect::TheZeroVector()andIntVect::TheUnitVector()can be used.Additional background
Using structured binding support of
amrex::GpuTuple, the following code should work (on GPU):Checklist
The proposed changes: