optimad / bitpit

Open source library for scientific HPC
http://optimad.github.io/bitpit/
GNU Lesser General Public License v3.0
115 stars 34 forks source link

Fix voloctree volume mapper #443

Closed marcocisternino closed 5 months ago

marcocisternino commented 5 months ago

In the _recoverPartition method of the voloctree volume mapper, particularly within the algorithm responsible for constructing the list of rank-local mapped mesh octants intended for transmission to the overlapping ranks of the reference mesh, there was an oversight in correctly re-initializing the mapped mesh octant Morton code within the main loop. Furthermore, a critical check was absent to halt the loop from inserting mapped mesh octants into the list once the mapped mesh rank reaches its final element.

andrea-iob commented 5 months ago

It's not much about the check (we can leave it as it is now), but having the discrepancy idx = nOctants and morton = morton of the (nOctant - 1) octant.

edoardolombardi commented 5 months ago

It's not much about the check (we can leave it as it is now), but having the discrepancy idx = nOctants and morton = morton of the (nOctant - 1) octant.

Yes, you're right, for this reason it makes sense. For the check I would keep for clarity the num octants or analogous.

marcocisternino commented 5 months ago

I splitted the algorithm filling the list of mapped octants to be sent to the reference mesh in 2 parts to make it more readable. Moreover, it should guarantee that no morton evaluation is called on non-existing ids. I tested in my cases using several number of processes.

marcocisternino commented 5 months ago

I renamed "first_overlap_ref_idx" in "first_overlap_map_idx". This remove the confusion about the meaning of the variable.

marcocisternino commented 5 months ago

It's not much about the check (we can leave it as it is now), but having the discrepancy idx = nOctants and morton = morton of the (nOctant - 1) octant.

It should be cleaner now, do you agree?

marcocisternino commented 5 months ago

I put this pull in draft. Further tests show problems in the splitted algorithm. At b59ef91 the algorithm works fine.

andrea-iob commented 5 months ago

I don't see the latest changes, can you check if you have uploaded them?

marcocisternino commented 5 months ago

As much as I could test it, it seems to work now.