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

AMR example and test coverage #255

Closed
wants to merge 14 commits into from
Closed

Conversation

simbilod
Copy link

Description of changes:

Adds a new section to the "cavity eigenmodes" example showing convergence behaviour with adaptive mesh refinement and enabling AMR test coverage (as discussed in #221)

  • New function in cavity.jl to setup the simulation with AMR
  • Analysis and plotting code in amr.jl; parameters are chosen for quick execution, but still showing the effect
  • New section in cavity.md documentation showcasing the plots
  • Config files, mesh files, new tests, fix bug in testcase to cover AMR

New doc section:

image

Issue #, if available:

Addresses #221

Disclaimer

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute
this contribution, under the terms of your choice.

@hughcars
Copy link
Collaborator

Hi @simbilod, not sure if you're still working on this but it looks like a great contribution to the documentation: it's nice to see amr gaining traction. I think we'll probably want to expand this in a few ways though before any merge:

  • It would be good to use amr on the cpw example, as they have interior boundary conditions. These have historically been a bigger stress test for AMR so would be a good place to demonstrate it rather than for the cavity problem where the smoothness of the solution means AMR should refine fairly uniformly.
  • The results of the simulation should be baselined and used for a regression example. This would mean deciding on some appropriate tolerance/max iteration for the AMR to ensure the job is small enough to run, but large enough to be interesting.
    The figures here already are looking great though, and Palace will be improved by their addition. If you have any questions or need guidance on any of that I'm happy to help.

@simbilod
Copy link
Author

Hi @simbilod, not sure if you're still working on this but it looks like a great contribution to the documentation: it's nice to see amr gaining traction. I think we'll probably want to expand this in a few ways though before any merge:

  • It would be good to use amr on the cpw example, as they have interior boundary conditions. These have historically been a bigger stress test for AMR so would be a good place to demonstrate it rather than for the cavity problem where the smoothness of the solution means AMR should refine fairly uniformly.
  • The results of the simulation should be baselined and used for a regression example. This would mean deciding on some appropriate tolerance/max iteration for the AMR to ensure the job is small enough to run, but large enough to be interesting.
    The figures here already are looking great though, and Palace will be improved by their addition. If you have any questions or need guidance on any of that I'm happy to help.

Hi @hughcars , thanks for the feedback! Yes showcasing a problem that benefits more from AMR is a good idea. I can take another crack in the coming week or so.

@simbilod
Copy link
Author

Closing to redo with provided feedback down the line

@simbilod simbilod closed this Aug 14, 2024
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