Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Concatenate changes comm #422

Closed
TheSlimvReal opened this issue Nov 28, 2019 · 2 comments · Fixed by #525
Closed

Concatenate changes comm #422

TheSlimvReal opened this issue Nov 28, 2019 · 2 comments · Fixed by #525
Labels
bug Something isn't working Question: Further information is requested

Comments

@TheSlimvReal
Copy link
Contributor

Description
When using the concatenate function, the comm of the result is eventually a different one than the comm of the input arrays. The comm will be changed when it's not ht.MPI_WORLD but another custom one. The concatenate function does not specify a comm internally and thus the ht.MPI_WORLD comm will be used for the result.

To Reproduce
Steps to reproduce the behavior:

a = ht.arange(23, split=0, comm=MPICommunication())
b = ht.zeros(2, comm=a.comm)
print('a', a.comm)
print('b', b.comm)

c = ht.concatenate((b, a))

print('c', c.comm)

Steps to reproduce the behavior:

  1. Which module/class/function is affected?
    concatenate in manipualtions
  2. What are the circumstances under which the bug appears?
    setting a custom communicator
  3. What is the exact error-message/errorous behavious?
    No error message, just unexpected behaviour.

Expected behavior
A comm of the input arrays should be used

Additional comments
I am not quite sure what the expected behaviour here would be.

@TheSlimvReal TheSlimvReal added bug Something isn't working Question: Further information is requested labels Nov 28, 2019
@coquelin77
Copy link
Member

coquelin77 commented Nov 29, 2019

there are 2 options for this.

  1. we leave it as it is. in this case the returned DNDarray has its own communicator and it is assumed that we are working on the whole world.
  2. we use the communicator from one of the inputs

I am partial to the first. i think that a new array should have its own communicator. however I have just noticed a different bug in cat: the inputs are unbalanced in the process of the code but they are not re-balanced at the end of the function

@Markus-Goetz
Copy link
Member

comms of a and b should be consistent. So it will be the comm of either one of the operators

Markus-Goetz added a commit that referenced this issue Apr 3, 2020
…mentation to state exceptions, changed exception types to consistent Python usage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Question: Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants