Skip to content

Fix EdgeDescriptor in parallel#123

Merged
UZerbinati merged 1 commit into
mainfrom
pbrubeck/fix/edgenr-parallel
Jul 3, 2026
Merged

Fix EdgeDescriptor in parallel#123
UZerbinati merged 1 commit into
mainfrom
pbrubeck/fix/edgenr-parallel

Conversation

@pbrubeck

@pbrubeck pbrubeck commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

@pbrubeck pbrubeck force-pushed the pbrubeck/fix/edgenr-parallel branch 2 times, most recently from 1da3c16 to 3416f9a Compare July 3, 2026 09:01
@pbrubeck pbrubeck force-pushed the pbrubeck/fix/edgenr-parallel branch from 3416f9a to d279d17 Compare July 3, 2026 09:50

@connorjward connorjward left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me. Those sizes may not be zero on all ranks.

@connorjward

Copy link
Copy Markdown
Collaborator

Does this need a test?

@pbrubeck

pbrubeck commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator Author

Does this need a test?

Those are the Firedrake tests that were previously broken

@connorjward

Copy link
Copy Markdown
Collaborator

Does this need a test?

Those are the Firedrake tests that were previously broken

Sure. But it's good practice to test things where they are implemented. It's a question of policy for @UZerbinati really.

@pbrubeck

pbrubeck commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator Author

Makes sense to me. Those sizes may not be zero on all ranks.

That's not the issue, we weren't hanging in parallel. It's just that we needed to make sure that we added boundary labels consistently across ranks. We needed to attach labels on the netgen mesh even if they were empty

@UZerbinati

Copy link
Copy Markdown
Collaborator

@connorjward Policy?

@UZerbinati UZerbinati merged commit f3a6ab3 into main Jul 3, 2026
15 checks passed
@connorjward

Copy link
Copy Markdown
Collaborator

Sure. But it's good practice to test things where they are implemented. It's a question of policy for @UZerbinati really.

As in, do you want people to add tests whenever they make changes to the code? It's a requirement for Firedrake for instance. Not bothered either way but I think it's important to think about.

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