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

Sort ghost indices in sub-dofmap based on original ordering #2930

Closed
wants to merge 2 commits into from

Conversation

jorgensd
Copy link
Sponsor Member

Relates to: #2881, but now in the context of usage in #2929.
Currently, collapsing a dofmap of a blocked space states that is does not reorder the degrees of freedom.
However, the ghost dofs are re-ordered, as the new index map is created with create_sub_map.

This PR adds in a sorting of the ghost indices, based on the indices of the initial ghosts.
It also relates to #2919.

@francesco-ballarin
Copy link
Member

So basically now we are moving to the position I used to have in #2881 (comment)?

I am fine either way, I am just asking because if we do guarantee that ghosts are now sorted as in the original ordering, I will beef up again my asserts 😛

@jorgensd
Copy link
Sponsor Member Author

So basically now we are moving to the position I used to have in #2881 (comment)?

I am fine either way, I am just asking because if we do guarantee that ghosts are now sorted as in the original ordering, I will beef up again my asserts 😛

Yes, it s either that or not use create_sub_map for collapsing spaces

@francesco-ballarin
Copy link
Member

@jorgensd @jpdean need to decide which one you'd like to get merged first between #2890 and #2930, since they are changing the same functionality, and then update the other PR.

@garth-wells
Copy link
Member

There are a few things to work through first, see #2929 (comment).

We want to aim for imposing as few constraints as reasonably possible, so ideally we don't want to force the re-ordering. If the caller needs a certain order then it should be possible for the caller to do the re-ordering.

@jorgensd
Copy link
Sponsor Member Author

There are a few things to work through first, see #2929 (comment).

We want to aim for imposing as few constraints as reasonably possible, so ideally we don't want to force the re-ordering. If the caller needs a certain order then it should be possible for the caller to do the re-ordering.

the points you have made in the referenced issue does not address the underlying issue, that if you create a sub index map of all indices, it ends up shuffling the ghosts.
The issue is that the shuffling of ghosts, when done on a collapsed map, completely changes the dofmap for the collapsed space of a blocked element, which does not make sense to me.
The re-ordering of a dofmap based on this index map is way more expensive than reordering prior to the «sub» index map construction.

@garth-wells
Copy link
Member

Superseded by #3091

@garth-wells garth-wells closed this Mar 7, 2024
@jorgensd jorgensd deleted the dokken/sort-submap-ghosts branch May 22, 2024 06:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants