Fix _sendrecv_get_destinations todo: loop only over neighbours

[max-models]

• Added _compute_neighbor_ranks(), computed once at __init__
• _sendrecv_get_destinations now checks neighbor ranks first, and only falls back to checking the rest if some markers remain unmatched
• self.markers[send_inds] and self._sorting_etas[send_inds] are now gathered once (see candidates, etas_to_send) instead of being re-gathered on every rank-loop iteration.

Improvements (Np=4,000,000):

• 4 ranks: 1.07x speedup
• 8 ranks: 1.34x speedup
• 16 ranks: 1.57x speedup

Merge after #348

[spossann]

Thanks for this. For testing, it would make a lot of sense to run the unit tests on 1, 2, 3 and 4 ranks, to test different topologies. However:

1. Some tests would probably have to be adapted, maybe even the default params file.
2. Would it be too costly to run the unit tests four times?

[max-models]

Thanks for this. For testing, it would make a lot of sense to run the unit tests on 1, 2, 3 and 4 ranks, to test different topologies. However:

1. Some tests would probably have to be adapted, maybe even the default params file.
2. Would it be too costly to run the unit tests four times?

Good questions, I'm worried that there will not be a big difference for 1-4 ranks since the number or ranks vs number of neighbours are the same. But I can try to come up with a benchmark.

[spossann]

Thanks for this. For testing, it would make a lot of sense to run the unit tests on 1, 2, 3 and 4 ranks, to test different topologies. However:

1. Some tests would probably have to be adapted, maybe even the default params file.
2. Would it be too costly to run the unit tests four times?

Good questions, I'm worried that there will not be a big difference for 1-4 ranks since the number or ranks vs number of neighbours are the same. But I can try to come up with a benchmark.

I was thinking more about topology: with 3 ranks you get one interior rank (without domain boundary), with 4 ranks you get a 2x2 square. Maybe such tests could be useful, however not specific to the neighbours. Anyhow this would be for a different PR.

[max-models]

Thanks for this. For testing, it would make a lot of sense to run the unit tests on 1, 2, 3 and 4 ranks, to test different topologies. However:

1. Some tests would probably have to be adapted, maybe even the default params file.
2. Would it be too costly to run the unit tests four times?

Good questions, I'm worried that there will not be a big difference for 1-4 ranks since the number or ranks vs number of neighbours are the same. But I can try to come up with a benchmark.

I was thinking more about topology: with 3 ranks you get one interior rank (without domain boundary), with 4 ranks you get a 2x2 square. Maybe such tests could be useful, however not specific to the neighbours. Anyhow this would be for a different PR.

Ah ok! Yes this could easily be done with --oversubscribe. But maybe it's enough to set up some unittests just for this function and test them with a few different configurations. Running all the unittests sounds like too much to me.

[spossann]

Thanks for this. For testing, it would make a lot of sense to run the unit tests on 1, 2, 3 and 4 ranks, to test different topologies. However:

1. Some tests would probably have to be adapted, maybe even the default params file.
2. Would it be too costly to run the unit tests four times?

Good questions, I'm worried that there will not be a big difference for 1-4 ranks since the number or ranks vs number of neighbours are the same. But I can try to come up with a benchmark.

I was thinking more about topology: with 3 ranks you get one interior rank (without domain boundary), with 4 ranks you get a 2x2 square. Maybe such tests could be useful, however not specific to the neighbours. Anyhow this would be for a different PR.

Ah ok! Yes this could easily be done with --oversubscribe. But maybe it's enough to set up some unittests just for this function and test them with a few different configurations. Running all the unittests sounds like too much to me.

Indeed, I would like to see all unit tests in pic/tests that are not marked with mpi_skip to be run on 1, 2, 3 and 4 ranks, respectively.

[max-models]

Thanks for this. For testing, it would make a lot of sense to run the unit tests on 1, 2, 3 and 4 ranks, to test different topologies. However:

1. Some tests would probably have to be adapted, maybe even the default params file.
2. Would it be too costly to run the unit tests four times?

Good questions, I'm worried that there will not be a big difference for 1-4 ranks since the number or ranks vs number of neighbours are the same. But I can try to come up with a benchmark.

I was thinking more about topology: with 3 ranks you get one interior rank (without domain boundary), with 4 ranks you get a 2x2 square. Maybe such tests could be useful, however not specific to the neighbours. Anyhow this would be for a different PR.

Ah ok! Yes this could easily be done with --oversubscribe. But maybe it's enough to set up some unittests just for this function and test them with a few different configurations. Running all the unittests sounds like too much to me.

Indeed, I would like to see all unit tests in pic/tests that are not marked with mpi_skip to be run on 1, 2, 3 and 4 ranks, respectively.

Like this? 6559d2d

[spossann]

Thanks for this. For testing, it would make a lot of sense to run the unit tests on 1, 2, 3 and 4 ranks, to test different topologies. However:

1. Some tests would probably have to be adapted, maybe even the default params file.
2. Would it be too costly to run the unit tests four times?

Good questions, I'm worried that there will not be a big difference for 1-4 ranks since the number or ranks vs number of neighbours are the same. But I can try to come up with a benchmark.

I was thinking more about topology: with 3 ranks you get one interior rank (without domain boundary), with 4 ranks you get a 2x2 square. Maybe such tests could be useful, however not specific to the neighbours. Anyhow this would be for a different PR.

Ah ok! Yes this could easily be done with --oversubscribe. But maybe it's enough to set up some unittests just for this function and test them with a few different configurations. Running all the unittests sounds like too much to me.

Indeed, I would like to see all unit tests in pic/tests that are not marked with mpi_skip to be run on 1, 2, 3 and 4 ranks, respectively.

Like this? 6559d2d

I think we need a new mark on the tests in pic/tests that we want to include, and then just test those with pytest -m new-mark ... for also 3 and 4 ranks.