search

envelope-project / laik

Other
9 stars 8 forks source link

WIP: Laik group get location #169

Closed Vinpasso closed 5 years ago

Vinpasso commented 5 years ago

Functionality to get the index into the world group from any index into a group that has world in its ancestors

Vinpasso commented 5 years ago

There is a basic test and the implementation itself is only a couple lines of code

weidendo commented 5 years ago

This implementation may be too simple: how do we ever add new processes? They will not be in any ancestor, if a group always has one parent with the top parent being the original world. It seems better to me to have an location array which is not a Laik_Group: groups should be immutable, while the location array should be able to be extended. So perhaps even simpler: maintain a toLocation[] array in each group (which is identity for original world).

It is nice that the test code itself is independent from the backend. However:

Vinpasso commented 5 years ago

how do we ever add new processes? They will not be in any ancestor, if a group always has one parent with the top parent being the original world.

It would be possible to add a new group as the ancestor of the original world which contains additional nodes.

It seems better to me to have an location array which is not a Laik_Group: groups should be immutable, while the location array should be able to be extended. So perhaps even simpler: maintain a toLocation[] array in each group (which is identity for original world).

Ok, I will implement this

the test itself only works with MPI installed (move the test below "mpi/")?

Can do.

would be good to also have a test using the TCP backend

I guess it can't hurt, ill add a second script for the tcp-backend.

Vinpasso commented 5 years ago

Status update:

It seems better to me to have an location array which is not a Laik_Group: groups should be immutable, while the location array should be able to be extended. So perhaps even simpler: maintain a toLocation[] array in each group (which is identity for original world).

Now implemented.

the test itself only works with MPI installed (move the test below "mpi/")?

Done.

would be good to also have a test using the TCP backend

I tried this. It turns out, the tcp backend does not have support for kv_store yet, so there is no tcp test for now.

weidendo commented 5 years ago

Thanks - I squashed the commits, and merged to master (used "git rebase -i" manually).

The exchange of the location string should not be optional, and not part of the LAIK API, but done in laik_init() and when new processes are added (not possible now). So I moved the sync function to the internal header.

I modified the test to use the location IDs, and moved the souces to "tests/src"...