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

General parser function for external fields #5349

Merged

Conversation

kli-jfp
Copy link
Contributor

@kli-jfp kli-jfp commented Oct 1, 2024

This feature replaces the InitializeExternalFieldsOnGridUsingParser with a more general function ComputeExternalFieldOnGridUsingParser. The new function takes parsed functions with four dimensions (x,y,z,t), so that it can be used to evaluate functions throughout the simulation, for example time-dependent external fields.

The function ComputeExternalFieldOnGridUsingParser has optional edge length and face areas.

Copy link
Member

@roelof-groenewald roelof-groenewald left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this PR! It will help to make things more maintainable when all grid parsing is done through the same function.

One question I have, do you think it would be possible to reduce the amount of code duplication in the two overloads of ComputeExternalFieldOnGridUsingParser? One suggestion would be to make the edge_lengths and face_areas arguments std::optional with default values of std::nullopt. That way if they are not given the lx, Sx, ... variables could just remain uninitialized and things like if(lx && ...) would return false.

@roelof-groenewald roelof-groenewald added the cleaning Clean code, improve readability label Oct 1, 2024
@kli-jfp kli-jfp force-pushed the general_field_parser_function branch from 64dbd99 to 9981ec7 Compare October 1, 2024 18:44
Copy link
Member

@roelof-groenewald roelof-groenewald 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 to me. I just have a question about the index change for the z-direction in 1d. See below.

Source/Initialization/WarpXInitData.cpp Outdated Show resolved Hide resolved
Source/Initialization/WarpXInitData.cpp Outdated Show resolved Hide resolved
Source/Initialization/WarpXInitData.cpp Outdated Show resolved Hide resolved
@kli-jfp kli-jfp changed the title [WIP] General parser function for external fields General parser function for external fields Oct 2, 2024
@kli-jfp kli-jfp changed the title General parser function for external fields [WIP] General parser function for external fields Oct 2, 2024
@kli-jfp
Copy link
Contributor Author

kli-jfp commented Oct 2, 2024

I'm struggling to get clang-tidy build to compile. Working on it.

Copy link
Member

@roelof-groenewald roelof-groenewald left a comment

Choose a reason for hiding this comment

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

The only remaining clang-tidy issues I see are these ones.

Source/Initialization/WarpXInitData.cpp Outdated Show resolved Hide resolved
Source/Initialization/WarpXInitData.cpp Outdated Show resolved Hide resolved
@roelof-groenewald roelof-groenewald self-assigned this Oct 2, 2024
@kli-jfp kli-jfp changed the title [WIP] General parser function for external fields General parser function for external fields Oct 2, 2024
Copy link
Member

@roelof-groenewald roelof-groenewald 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! Thanks for the contribution!

@roelof-groenewald roelof-groenewald merged commit 841d7ee into ECP-WarpX:development Oct 2, 2024
37 checks passed
@kli-jfp kli-jfp deleted the general_field_parser_function branch October 2, 2024 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleaning Clean code, improve readability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants