dash
dash copied to clipboard
removed unnecessary inheritance
bug fix for issue #631
Codecov Report
Merging #632 into development will decrease coverage by
0.13%
. The diff coverage isn/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% <ø> (ø) |
:arrow_up: |
dash/include/dash/internal/Logging.h | 68.18% <0%> (-31.82%) |
:arrow_down: |
dash/test/TestBase.h | 68.18% <0%> (-22.73%) |
:arrow_down: |
dash/include/dash/Team.h | 83.15% <0%> (-9.48%) |
:arrow_down: |
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.
RegionSpec is not really "a" Dimensional. The member RegionCoords is Dimensional. But RegionSpec only combines RegionCoords with some other properties.
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.
What is the way forward here? I don't remember this was discussed at the F2F but it keep biting...
i still think that no inheritance should be used. I don't use the data structure of the base class or any methods.
Can the RegionSpec
be implemented in terms of Dimensional
?
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.
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?
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.
@fuchsto I think we should merge this PR as is. Can you accept RegionSpec
not being a Dimensional
?