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

Perfect Periodic BC #266

Merged
merged 33 commits into from
Sep 4, 2024
Merged

Perfect Periodic BC #266

merged 33 commits into from
Sep 4, 2024

Conversation

LeilaGhaffari
Copy link
Contributor

@LeilaGhaffari LeilaGhaffari commented Jun 13, 2024

This PR is aiming at adding support for perfect periodic BC.
I have added two meshes in examples/cube/. cube.mesh is a regular cube and cube_periodic.mesh is periodic.

@sebastiangrimberg , are the boundary conditions specified here and here making sense?

Edit: Just noticed mfem/data/periodic-cube.mesh and it seems to be different than the one I created. This one fails too though.

@LeilaGhaffari LeilaGhaffari force-pushed the lgh/periodic-bc branch 3 times, most recently from 679e555 to 41beca2 Compare July 25, 2024 21:10
@LeilaGhaffari LeilaGhaffari force-pushed the lgh/periodic-bc branch 2 times, most recently from d40d53e to 185a215 Compare August 6, 2024 16:57
@LeilaGhaffari LeilaGhaffari force-pushed the lgh/periodic-bc branch 4 times, most recently from a0cfbf3 to 6671aa1 Compare August 13, 2024 18:24
@LeilaGhaffari LeilaGhaffari changed the title [WIP] Perfect Periodic BC Perfect Periodic BC Aug 13, 2024
@LeilaGhaffari
Copy link
Contributor Author

@hughcars , I don't seem to have necessary permissions to add reviewers but I think this is in good shape for review. I would still need to add support for lexographic elements but that shouldn't block the review. Thanks!

@hughcars hughcars self-requested a review August 14, 2024 16:40
@LeilaGhaffari LeilaGhaffari force-pushed the lgh/periodic-bc branch 4 times, most recently from 0b81873 to 4ab35ea Compare August 16, 2024 19:27
Copy link
Collaborator

@hughcars hughcars left a comment

Choose a reason for hiding this comment

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

Almost done. The issue with the hex/prism/tet waveguide is going to mean the gmsh meshes need to be refined, cavity rebaselined and then the tables of the corresponding modes filled in. Additionally, do not need to remove the attributes on donor and receiver (and can patch mfem to not do the same as well) then we can add computing the power flux for each mode to the regression tests.

examples/cylinder/mesh/mesh.jl Outdated Show resolved Hide resolved
test/examples/runtests.jl Outdated Show resolved Hide resolved
palace/utils/geodata.cpp Outdated Show resolved Hide resolved
test/examples/runtests.jl Show resolved Hide resolved
palace/utils/geodata.cpp Outdated Show resolved Hide resolved
palace/utils/geodata.cpp Outdated Show resolved Hide resolved
docs/src/examples/cylinder.md Outdated Show resolved Hide resolved
- Improves geometry fidelity and avoids issue in mfem where
periodicity fails for meshes with too few elements in the
periodic face.
- Change geodata.cpp to not erase boundary attributes on periodic faces.
Additionally to not fail if an incoming mesh has an interior boundary.
- Regenerate baseline test data based on the updated meshes.
- Update documentation to use Teflon in waveguide
- Fixes issue with underspecified Q for waveguide
- Makes documentation cleaner compared to cavity
@hughcars
Copy link
Collaborator

hughcars commented Sep 3, 2024

I've addressed all the issues here in https://github.com/awslabs/palace/tree/hughcars/periodic-bc including merging main in. I'm not sure how to merge those commits onto this branch, given it's originally from a fork. You can either merge them in, or I can open another PR from that branch. It should just be a fast forward from this branch.

@LeilaGhaffari
Copy link
Contributor Author

Thanks @hughcars for fixing the issues. I just pushed your fixes to this branch. Is there anything else I can help with before merging?

@hughcars
Copy link
Collaborator

hughcars commented Sep 4, 2024

Thanks @hughcars for fixing the issues. I just pushed your fixes to this branch. Is there anything else I can help with before merging?

I just read through it again and found a few small things I missed, there's one more commit on hughcars/periodic-bc then we're good to merge.

@LeilaGhaffari
Copy link
Contributor Author

Thanks @hughcars for fixing the issues. I just pushed your fixes to this branch. Is there anything else I can help with before merging?

I just read through it again and found a few small things I missed, there's one more commit on hughcars/periodic-bc then we're good to merge.

Just pushed it, thanks!

@hughcars hughcars enabled auto-merge September 4, 2024 20:57
@hughcars hughcars merged commit 4563990 into awslabs:main Sep 4, 2024
17 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