Added function to create std::variant for multiple array views.

[odlomax]

During PR #218, I found myself writing two nested switch statements in order to create an ArrayView from an Array. I've found myself writing this same pattern multiple times throughout Atlas, and indeed other external applications which use atlas.

This PR adds provides an alternative to the nested switch statement. The method util::make_array_view_variant<Values<types...>, Ranks<ints...>>(Array& array) creates an std::variant of ArrayView types, and then assigns the view that matches array (so long as the value-type and rank can be found within the Values and Ranks arguments respectively. The ArrayView can then be accessed rather succinctly using std::visit.

[wdeconinck]

Hi @odlomax I think this is a great idea.
I was wondering if this should not become part of the "array" namespace. If you want we could discuss offline.
It should also cater for device views though.
array::make_host_view_variant and array::make_device_view_variant
All possibly supported Values and Ranks could be immediately part of the variant type, removing even more boiler-plate?

[odlomax]

Hi @odlomax I think this is a great idea. I was wondering if this should not become part of the "array" namespace. If you want we could discuss offline. It should also cater for device views though. array::make_host_view_variant and array::make_device_view_variant All possibly supported Values and Ranks could be immediately part of the variant type, removing even more boiler-plate?

Hi @wdeconinck,

Thanks for the feedback! I'm on leave at the moment, but this is oddly recreational between baby's naps. I'll arrange a catch up when I'm back after next week. 🙂

[odlomax]

Hi @wdeconinck. I believe I've incorporated all of your suggestions. It's ready for a look when you have the time. Cheers!

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 92.96875% with 9 lines in your changes missing coverage. Please review.

Project coverage is 80.02%. Comparing base (afd3b5f) to head (73d990f).
Report is 14 commits behind head on develop.

Files with missing lines	Patch %	Lines
src/tests/array/test_array_view_variant.cc	92.70%	7 Missing ⚠️
src/atlas/array/ArrayViewVariant.cc	93.75%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #220      +/-   ##
===========================================
+ Coverage    79.95%   80.02%   +0.06%     
===========================================
  Files          792      794       +2     
  Lines        62309    62491     +182     
===========================================
+ Hits         49822    50011     +189     
+ Misses       12487    12480       -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[odlomax]

@wdeconinck Apologies for the last minute push.

There were relatively important EXPECT statements that needed to be added to lines 139 and 164 of the test. I also removed some extraneous whitespace at the end of an otherwise untouched file.

[odlomax]

Okay, I won't touch this until it's reviewed. I promise!

[odlomax]

Hi @wdeconinck,

When attempting to using this in anger (see PR #226), I found that it was worth refactoring the make_view_variant methods to produce either const or non-const versions of the variant depending on the constness of the array. Shall I merge the refactor into this PR, or leave it in #226?

[odlomax]

Hi @wdeconinck @sbrdar,

I've refactored the introspection helpers as we discussed on Slack. I've also added an additional helper to distinguish between value_type and non_const_value_type. Seemed like a sensible idea.

Cheers!

[odlomax]

@wdeconinck the latest push has your suggested signature for the introspection helpers. Thanks for the suggestion, it's a lot tidier.

[odlomax]

Basic GHA is passing now!

https://github.com/JCSDA-internal/atlas/actions/runs/11157940876

[odlomax]

MacOS tests! Nooo!

[wdeconinck]

MacOS tests! Nooo!

https://github.com/ecmwf/atlas/actions/runs/11157941275/job/31068813417?pr=220 should not be the problem of this PR, but about the CI infrastructure. I passed on this problem.

[wdeconinck]

This looks great now! Thanks!