Convert IntVect to IntVectND

[AlexanderSinn]

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 functions IntVectCat, IntVectSplit and IntVectResize were added to IntVectND.

I didn't find a way to make IntVect::Zero and IntVect::Unit work with n dimensions, so I removed them (converting them to static constexpr or inline static doesn't work with MSVC). Instead, IntVect::TheZeroVector() and IntVect::TheUnitVector() can be used.

Additional background

Using structured binding support of amrex::GpuTuple, the following code should work (on GPU):

int i, j, k, n;
...
// amrex::IntVectND<4>
amrex::IntVectND iv1 (i, j, k, n);
// amrex::IntVectND<8>
amrex::IntVectND iv2 = amrex::IntVectCat(iv1, iv1);
// ... = amrex::GpuTuple<amrex::IntVectND<3>,amrex::IntVectND<2>,amrex::IntVectND<2>,amrex::IntVectND<1>>
auto [iv3, iv4, iv5, iv6] = amrex::IntVectSplit<3,2,2,1>(iv2); 
// int, int, int = amrex::IntVectND<3>
auto [i2, j2, k2] = iv3;
// int = amrex::IntVectND<1>
auto [n2] = iv6;

assert(i==i2 && j==j2 && k==k2 && n==n2);

Checklist

The proposed changes:

• fix a bug or incorrect behavior in AMReX
• add new capabilities to AMReX
• changes answers in the test suite to more than roundoff level
• are likely to significantly affect the results of downstream AMReX users
• include documentation in the code and/or rst files, if appropriate

[WeiqunZhang]

Should we have extern template class IntVectND<AMREX_SPACEDIM>, which might help reduce compile time?

[AlexanderSinn]

Wouldn't that prevent the member functions from getting inlined, which could be bad when IntVect is used in a parallel for?

[WeiqunZhang]

My understanding is it should not affect run time performance. But I might be wrong.

[WeiqunZhang]

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: ***@***.***>

[AlexanderSinn]

I tried to add extern template class IntVectND<AMREX_SPACEDIM>;however, without adding template class IntVectND<AMREX_SPACEDIM>;to the cpp file. I expected to get a linker error however it compiled just fine.

[WeiqunZhang]

Maybe it's because all those functions have already been inlined.

[WeiqunZhang]

diff --git a/Src/Base/AMReX_IntVect.H b/Src/Base/AMReX_IntVect.H
index 70a12811a2..7c472742ae 100644
--- a/Src/Base/AMReX_IntVect.H
+++ b/Src/Base/AMReX_IntVect.H
@@ -710,6 +710,9 @@ public:
         return IntVectND<dim>(std::numeric_limits<int>::lowest());
     }
 
+    static const IntVectND<dim> Zero;
+    static const IntVectND<dim> Unit;
+
     AMREX_GPU_HOST_DEVICE AMREX_FORCE_INLINE
     static constexpr std::size_t size () noexcept {
         return static_cast<std::size_t>(dim);
@@ -767,6 +770,13 @@ private:
     int vect[dim] = {};
 };
 
+
+template <>
+inline constexpr IntVectND<AMREX_SPACEDIM> IntVectND<AMREX_SPACEDIM>::Zero{0};
+
+template <>
+inline constexpr IntVectND<AMREX_SPACEDIM> IntVectND<AMREX_SPACEDIM>::Unit{1};
+
 // Template deduction guide for IntVectND
 template<std::size_t dim>
 AMREX_GPU_HOST_DEVICE // __device__ for HIP

This seems to work. So we could get IntVect::Zero and IntVect::Unit back for backward compatibility.

Unfortunately template <int dim> inline constexpr IntVectND<dim> IntVectND<dim>::Zero{0} might only work for gcc. https://stackoverflow.com/questions/72409091/define-a-static-constexpr-member-of-same-type-of-a-template-class

[WeiqunZhang]

Unfortunately, nvcc does not work.

[AlexanderSinn]

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 static constexpr.

[WeiqunZhang]

That may not work in cases like

IntVect::Zero + sign(a_side) * 2 * BASISV(idir);

This is copied from a real code.

Maybe we can try to figure out how to make nvcc happy. The error message is

/home/runner/work/amrex/amrex/Src/Base/AMReX_IntVect.H:775:34: error: conflicting declaration ‘amrex::IntVectND<3> amrex::IntVectND<3>::Zero’
  775 | inline constexpr IntVectND<AMREX_SPACEDIM> IntVectND<AMREX_SPACEDIM>::Zero{0};
      |                                  ^~~~~~~~~~~~~
/home/runner/work/amrex/amrex/Src/Base/AMReX_IntVect.H:713:24: note: previous declaration as ‘const amrex::IntVectND<3> amrex::IntVectND<3>::Zero’
  713 |     static const IntVectND<dim> Zero;
      |                        ^~~~

[WeiqunZhang]

Maybe it wants constexpr const?

[AlexanderSinn]

It seems to work if both are just const with no constexpr.

[WeiqunZhang]

Great. Maybe it will even work for all <int dim>.

[AlexanderSinn]

constexpr const actually works somehow

[WeiqunZhang]

@ax3l This PR will break pyamrex. How do you want to handle this? Should we wait till pyamrex has a PR ready?

[ax3l]

Thanks for the ping. That is all we need to track.

Please go ahead here to merge and we update pyAMReX shortly after.

[ax3l]

pyAMReX update coming via AMReX-Codes/pyamrex#332 :)