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

Assign ranks to leaves of binary partition in depth-first order #1681

Merged
merged 3 commits into from
Jul 17, 2021

Conversation

oskooi
Copy link
Collaborator

@oskooi oskooi commented Jul 16, 2021

Fixes the bug described in #1673 (comment).

With this change, the unit test added in #1673 for test_chunk_layout.py passes and thus #1673 needs only to be rebased after this is merged.

@oskooi oskooi added the bug label Jul 16, 2021
@stevengj
Copy link
Collaborator

Given this change it seems like you should also change

const int proc = (!_bp) ? i * count_processors() / chunk_volumes.size() : ids[i] % count_processors();

to simply:

const int proc = ids[i] % count_processors();

both for simplicity and to ensure that the chunk process id is set consistently with the value in the binary tree.

@oskooi
Copy link
Collaborator Author

oskooi commented Jul 17, 2021

Good suggestion. I have made the change.

@stevengj stevengj merged commit e7acd2f into NanoComp:master Jul 17, 2021
@oskooi oskooi deleted the binary_partition_proc_ids branch July 17, 2021 23:31
bencbartlett pushed a commit to bencbartlett/meep that referenced this pull request Sep 9, 2021
…Comp#1681)

* assign ranks to leaves of binary partition in depth-first order

* ensure proc_id is not larger than number of processors

* simplify setting of chunk process ID
mawc2019 pushed a commit to mawc2019/meep that referenced this pull request Nov 3, 2021
…Comp#1681)

* assign ranks to leaves of binary partition in depth-first order

* ensure proc_id is not larger than number of processors

* simplify setting of chunk process ID
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants