removed unnecessary inheritance

[dhinf]

bug fix for issue #631

[codecov]

Codecov Report

Merging #632 into development will decrease coverage by 0.13%.
The diff coverage is n/a.

@@               Coverage Diff               @@
##           development     #632      +/-   ##
===============================================
- Coverage        84.95%   84.81%   -0.14%     
===============================================
  Files              335      335              
  Lines            24798    24798              
  Branches         11249    11262      +13     
===============================================
- Hits             21066    21033      -33     
  Misses            3729     3729              
- Partials             3       36      +33

Impacted Files	Coverage Δ
dash/include/dash/halo/Halo.h	97.61% <ø> (ø)	⬆️
dash/include/dash/internal/Logging.h	68.18% <0%> (-31.82%)	⬇️
dash/test/TestBase.h	68.18% <0%> (-22.73%)	⬇️
dash/include/dash/Team.h	83.15% <0%> (-9.48%)	⬇️

[bertwesarg]

Could you test it with Intel 19? I could send this one-liner to the original report, but it may take some time to get feedback.

[fuchsto]

The subclassing was not unnecessary, it indicates the type category.
This is why public inheritance was used. The formal reading is "RegionSpec IS_A Dimensional<uint8_t, D>".

The compiler error is right, though, the fix is to add missing delegate constructor calls in RegionSpec.

[dhinf]

RegionSpec is not really "a" Dimensional. The member RegionCoords is Dimensional. But RegionSpec only combines RegionCoords with some other properties.

[fuchsto]

Well, the question whether "A is a B" (A : public B) means:
"Can you pass an instance of A to every context that expects a B?
In this example, let's assume:

SizeT volume(const Dimensional & d) { return std::accumulate( ... , std::multiplies); }
RegionSpec<...> rs(...);
auto region_volume = volume(rs);

I don't see why something like that should not be okay, so RegionSpec could act as Dimensional.
Back then, I added some other types for Cartesian domains, perhaps something else is a better fit
We'll see tomorrow.

[devreal]

What is the way forward here? I don't remember this was discussed at the F2F but it keep biting...

[dhinf]

i still think that no inheritance should be used. I don't use the data structure of the base class or any methods.

[devreal]

Can the RegionSpec be implemented in terms of Dimensional?

[dhinf]

Not really, because RegionSpec combines RegionCoords (Dimensional), the extent and some other useful things. In an older version i didn't used RegionCoords, but instead Dimensional to store the coordinates. Later i devided them. And at this point i missed to delete the inheritance.

[devreal]

In an older version i didn't used RegionCoords, but instead Dimensional to store the coordinates

What's the reason to not use Dimensional to store the coordinates anymore?

[dhinf]

Because i needed RegionCoords separately for other components. But seriously, i don't understand the problem here. We have so much other, more urgent, stuff to do.

[devreal]

@fuchsto I think we should merge this PR as is. Can you accept RegionSpec not being a Dimensional?