Add reduction for arrays.

[pratikvn]

This PR adds some reduction kernels for arrays. For CUDA and HIP the existing reduction kernels are reduced, with only one kernel added.

TODO

• Add a return overload.

[yhmtsai]

I want to ask whether we make Array class related to the device (not header only) or not? (before release 1.4)
for now, we have fill and this pr reduce.

• have fill and reduce in class:
  good for usage,
  but need to link all ginkgo always and still use free function to use fill/reduce in the device side
• only have fill and reduce free function
  Array is header-only class
  but always use free function to call fill/reduce.

[upsj]

Generally, I like to think of gko::Array as a the only vocabulary type that Ginkgo exposes. Something that can be used universally for interoperability with other libraries and user data. Now we have a few additions, namely value-converting Array::operator= and Array::fill that rely on actual device kernel and only work for specific ValueTypes. The more I think about it, the more I think it might be better to keep these out (replacing them by explicit conversion kernels for arrays only used internally, and maybe a datatype-agnostic Array::fill that acts like repeated memcpy)
All of this becomes especially important in the context of #715, where Array is the fundamental type that gets passed around between all shared libraries.
My point here is: If people want to compute a reduction over an array, I believe they should use more high-level functionality, like Dense or (maybe later) the simple kernel interface. Otherwise we risk overloading the Array type.

[pratikvn]

I agree with not overloading Array, but I think that can severely restrict functionality. For example, the only way to store integer types right now is with arrays. And there are many situations where you need reductions for these integer arrays.

[upsj]

In that case, wouldn't a free function ValueType reduce(const Array<ValueType>&) be more suitable? What use case do you have in mind for integer reductions from the user side?

[pratikvn]

Yes, I guess we could add a free function instead of member functions. I am not sure where to add them though. I think there are multiple use cases for reduction over integer arrays. To calculate the total number of non-zeros in a matrix, if you have the number of nonzeros per row to give a very specific example :)

But in general, I believe this is something useful not only from a user's perspective but also for us to write our algorithms more easily in our core.

[upsj]

In core algorithms, we usually need prefix sums, not plain reductions. And these computations are mostly necessary in the context of a matrix conversion, which we represent with a high-level interface already. If we want to use this operation inside core, we can use existing kernels.

I think we need to find a balance of how much we want to enable to write their own complex algorithms, and how generic we want to make that. Sum reductions are for example not the only important operation, think of (arg)min/max for building ELL.

[codecov]

Codecov Report

Merging #831 (69de7a4) into develop (83bd858) will decrease coverage by 0.00%.
The diff coverage is 97.29%.

@@             Coverage Diff             @@
##           develop     #831      +/-   ##
===========================================
- Coverage    94.73%   94.72%   -0.01%     
===========================================
  Files          431      434       +3     
  Lines        35669    35706      +37     
===========================================
+ Hits         33790    33822      +32     
- Misses        1879     1884       +5     

Impacted Files	Coverage Δ
core/device_hooks/common_kernels.inc.cpp	0.00% <0.00%> (ø)
include/ginkgo/core/base/array.hpp	94.85% <ø> (ø)
common/unified/components/reduce_array.cpp	100.00% <100.00%> (ø)
core/base/array.cpp	100.00% <100.00%> (ø)
omp/test/components/reduce_array.cpp	100.00% <100.00%> (ø)
reference/components/reduce_array.cpp	100.00% <100.00%> (ø)
reference/test/base/array.cpp	100.00% <100.00%> (ø)
omp/reorder/rcm_kernels.cpp	94.44% <0.00%> (-3.09%)	⬇️
core/base/extended_float.hpp	92.23% <0.00%> (+0.97%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 83bd858...69de7a4. Read the comment docs.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

88.9% Coverage
0.0% Duplication

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

85.0% Coverage
0.0% Duplication

[yhmtsai]

need another name for reduce2 and how many changes will be related to #833

[thoasm]

Mostly minor comments, but some of them I would like to have resolved before approving.

[thoasm]

LGTM!

stale

[yhmtsai]

LGTM in general.
but some nit on the test case

[upsj]

LGTM! For #885 you could still use prefix_sum to avoid duplicate computations, but this is useful regardless. If you want, you can also try using #833 to unify the kernel implementations.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information