Bug/568 get halo behaviour if no data on process

[lucaspataro]

Description

Issue/s resolved: #568

Empty ranks at the end of the communicator are now ignored.

Type of change

• Bug fix (non-breaking change which fixes an issue)

Due Diligence

• All split configurations tested
• Documentation updated (if needed)

Does this change modify the behaviour of other functions? If so, which?

no

[codecov]

Codecov Report

Merging #631 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #631   +/-   ##
=======================================
  Coverage   97.38%   97.39%           
=======================================
  Files          87       87           
  Lines       16758    16802   +44     
=======================================
+ Hits        16320    16364   +44     
  Misses        438      438           

Impacted Files	Coverage Δ
heat/core/dndarray.py	96.58% <100.00%> (+0.04%)	⬆️
heat/core/tests/test_dndarray.py	100.00% <100.00%> (ø)
heat/core/tests/test_statistics.py	100.00% <100.00%> (ø)

---

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 7cee8f0...5f063ad. Read the comment docs.

[ClaudiaComito]

You want to be checking if a DNDarray is balanced here. Otherwise you could hypothetically have 2 empty processes and then some data on one of the next ones.
On the other hand, if a DNDarray is balanced and lshape[split] on rank is 0, you don't need to check the next rank.

[lucaspataro]

@ClaudiaComito What is the desired behavior if there are empty ranks in the middle? Preventing the halo all together, or skipping the empty ranks and using the next/prev non-empty rank?

[ClaudiaComito]

Hi @lucaspataro, that's an interesting question. I think so far we're assuming that operations are performed on balanced DNDarrays, in that case the empty ranks will always be at the end. I would say that for now an empty rank "in the middle" is an edge case we don't really need to worry about. What do you think, @krajsek ?

[lucaspataro]

Hi @ClaudiaComito, i added a RuntimeError if tensor is not balanced and removed the check for empty next rank like you suggested.

[ClaudiaComito]

This looks good to me, thanks for fixing this. I have updated test_percentile to include a test with no data on process.

I have one change request.

[coquelin77]

bump, requested changes and conflict resolution

[mtar]

GPU cluster tests are currently disabled on this Pull Request.

[ClaudiaComito]

Great, thanks a lot!