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

Preserve default mesh name after Netgen mesh creation #3951

Conversation

MohamedAlyLoutfy
Copy link
Contributor

…pointing errors

Description

Root Cause:
The function createFromTopology in ngsPETSc.FiredrakeMesh sets the mesh's name to "Default" regardless of the provided custom or default name. This overwrites the expected mesh.name set during the Firedrake Mesh creation.
Fix Summary:
To preserve the expected mesh.name:
After createFromTopology is called, explicitly reset mesh.name to the desired value (name) to ensure the consistency of the naming conventions.
This ensures that the custom or default name (DEFAULT_MESH_NAME, i.e., "firedrake_default") is maintained after the Netgen mesh is created.

@MohamedAlyLoutfy MohamedAlyLoutfy changed the title Preserve default mesh name after Netgen mesh creation #3945 Preserve default mesh name after Netgen mesh creation Jan 4, 2025
@MohamedAlyLoutfy
Copy link
Contributor Author

MohamedAlyLoutfy commented Jan 4, 2025

Hello @nbouziani @dham I have created a fix for this issue in this PR, i made it as a draft PR until the CI check passes, I'm waiting approval for the workflows so i can make it ready for review, Thank you.

firedrake/mesh.py Outdated Show resolved Hide resolved
Copy link
Contributor

@connorjward connorjward left a comment

Choose a reason for hiding this comment

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

Thanks. I am still a bit confused about what is going on. Would the following changes work? They would then be consistent with what we do for the non-netgen case.

firedrake/mesh.py Outdated Show resolved Hide resolved
firedrake/mesh.py Outdated Show resolved Hide resolved
Copy link
Contributor

@connorjward connorjward left a comment

Choose a reason for hiding this comment

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

Looks good now. Thanks!

@connorjward
Copy link
Contributor

@MohamedAlyLoutfy I assume that this is ready to merge and no longer a draft? If so please mark as such.

@MohamedAlyLoutfy MohamedAlyLoutfy marked this pull request as ready for review January 15, 2025 14:13
@MohamedAlyLoutfy
Copy link
Contributor Author

@MohamedAlyLoutfy I assume that this is ready to merge and no longer a draft? If so please mark as such.

yes, i made it ready for review so it's no more a draft @connorjward

@connorjward connorjward enabled auto-merge (squash) January 15, 2025 14:15
@connorjward connorjward disabled auto-merge January 15, 2025 15:34
@connorjward connorjward merged commit cbfc2b2 into firedrakeproject:master Jan 15, 2025
14 of 18 checks passed
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.

2 participants