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

Fix the extrapolation for guard cells in the insulator region #5499

Merged

Conversation

dpgrote
Copy link
Member

@dpgrote dpgrote commented Dec 6, 2024

For the nodal guard cells in the insulator region, the extrapolation was being done relative to cells only inside the domain, not using the value on the boundary. This fixes this to do extrapolation from the value on the boundary and the next inward cell.

@dpgrote dpgrote added the component: boundary PML, embedded boundaries, et al. label Dec 6, 2024
Copy link
Member

@EZoni EZoni left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me. How did you catch this and/or do you think we can change something on the CI end to make our tests more sensitive to errors like this? It seems like this fix does not affect the result of any tests currently.

@EZoni EZoni self-assigned this Dec 6, 2024
@dpgrote
Copy link
Member Author

dpgrote commented Dec 7, 2024

Thanks, looks good to me. How did you catch this and/or do you think we can change something on the CI end to make our tests more sensitive to errors like this? It seems like this fix does not affect the result of any tests currently.

I caught this debugging the changes in the PR #5475. I was seeing wrong values in the guard cells. Adding a CI for this would be a bit of work since it only affects the data in the guard cells. Can the guard cells be included in the benchmark values? It doesn't seem like it. I would have to come up with a case where there are particles near the boundary that are affected by the data in the guard cells because of their shape function, which would be a sizable amount of work.

@EZoni
Copy link
Member

EZoni commented Dec 7, 2024

Can the guard cells be included in the benchmark values? It doesn't seem like it.

Thanks @dpgrote, not sure about this, but we could look into it, it's a good point.

@EZoni EZoni added bug Something isn't working bug: affects latest release Bug also exists in latest release version labels Dec 7, 2024
@EZoni EZoni merged commit c705575 into ECP-WarpX:development Dec 7, 2024
37 checks passed
@dpgrote dpgrote deleted the fix_PEC_insulator_field_extrapolation branch December 7, 2024 01:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug: affects latest release Bug also exists in latest release version bug Something isn't working component: boundary PML, embedded boundaries, et al.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants