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

Fix voloctree volume mapper #443

Merged
merged 4 commits into from
Feb 8, 2024
Merged

Fix voloctree volume mapper #443

merged 4 commits into from
Feb 8, 2024

Conversation

marcocisternino
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member Author

marcocisternino commented Jan 25, 2024

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 marcocisternino force-pushed the fix.voloctree.mapper branch 2 times, most recently from 0b23652 to ad8664d Compare January 29, 2024 13:25
@marcocisternino
Copy link
Member Author

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

@marcocisternino
Copy link
Member Author

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
Copy link
Member Author

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

@andrea-iob
Copy link
Member

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

@marcocisternino marcocisternino force-pushed the fix.voloctree.mapper branch 2 times, most recently from 5af6de8 to cb1685d Compare February 2, 2024 16:50
@marcocisternino marcocisternino marked this pull request as ready for review February 2, 2024 16:50
@marcocisternino
Copy link
Member Author

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

src/voloctree/voloctree_mapper.cpp Outdated Show resolved Hide resolved
@marcocisternino marcocisternino merged commit 56775c4 into master Feb 8, 2024
6 checks passed
@marcocisternino marcocisternino deleted the fix.voloctree.mapper branch February 8, 2024 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants