-
Notifications
You must be signed in to change notification settings - Fork 196
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
Add QED lookup tables generator/reader in /Tools #3137
Conversation
Docs/source/usage/workflows/generate_lookup_tables_with_tools.rst
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've only reviewed to C++
and sh
parts and they look good to me (I'll let @ax3l review in more details the github workflows and cmake parts).
Only significant comment is that it would be good to provide somewhere recommended table parameters for users (this can be done in a follow-up PR though).
Docs/source/usage/workflows/generate_lookup_tables_with_tools.rst
Outdated
Show resolved
Hide resolved
--dndt_chi_min [DOUBLE] minimum chi parameter for the dNdt table | ||
--dndt_chi_max [DOUBLE] maximum chi parameter for the dNdt table | ||
--dndt_how_many [INTEGR] number of points in the dNdt table | ||
--pair_chi_min [DOUBLE] minimum chi for the pair production table (BW only) | ||
--pair_chi_max [DOUBLE] maximum chi for the pair production table (BW only) | ||
--pair_chi_how_many [INTEGR] number of chi points in the pair production table (BW only) | ||
--pair_frac_how_many [INTEGR] number of frac points in the pair production table (BW only) | ||
--em_chi_min [DOUBLE] minimum chi for the photon emission table (QS only) | ||
--em_chi_max [DOUBLE] maximum chi for the photon emission production table (QS only) | ||
--em_frac_min [DOUBLE] minimum frac for the photon emission production table (QS only) | ||
--em_chi_how_many [INTEGR] number of chi points in the photon emission table (QS only) | ||
--em_frac_how_many [INTEGR] number of frac points in the photon emission table (QS only) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be useful for users to give recommended table parameters in this file (there are a lot of parameters and it's not trivial to choose them).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are right... we need to properly document everything related to the QED module... I promise that I will work on that starting from January 2023!
@ax3l , what do you think about the cmake and the github workflow parts of this PR? |
Docs/source/usage/workflows/generate_lookup_tables_with_tools.rst
Outdated
Show resolved
Hide resolved
Docs/source/usage/parameters.rst
Outdated
@@ -2724,6 +2724,8 @@ Lookup tables store pre-computed values for functions used by the QED modules. | |||
|
|||
* ``qed_bw.save_table_in`` (`string`): where to save the lookup table | |||
|
|||
Alternatively, the lookup table can be generated using a standalone tool (see workflows section). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a link to the specific workflow sub-section?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be ok now
This is very convenient, thank you for implementing this tool, @lucafedeli88! As @NeilZaim mentioned, it would be useful to provide a guide for the choice of the parameters. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CodeQL found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.
2561597
to
ea85181
Compare
Tools/QedTablesUtils/CMakeLists.txt
Outdated
set(arg_parser_cpp "Source/ArgParser/QedTablesArgParser.cpp") | ||
set(arg_parser_h "Source/ArgParser/QedTablesArgParser.H") | ||
|
||
add_library( QedToolsArgParser ${arg_parser_cpp} ${arg_parser_h} ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this library used anywhere else?
If you like, generalize it and make it part of the ABLASTR utils? (as a follow-up PR?)
@ax3l , it seems to me that when WarpX is compiled as a library the QED tools are not built. |
@@ -12,6 +12,7 @@ This section collects typical user workflows and best practices for WarpX. | |||
workflows/domain_decomposition | |||
workflows/plot_distribution_mapping | |||
workflows/debugging | |||
workflows/generate_lookup_tables_with_tools.rst |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.rst
ending not needed :)
#4804
Follow-up to ECP-WarpX#3137
This PR adds new tools to generate and/or read into human-readable format the QED lookup tables (they can be compiled with
cmake
by enabling the flagDWarpX_QED_TOOLS
. The PR also adds a documentation page to succinctly describe how to use these tools. Finally, it adds a new CI test to check that these tools generate lookup tables identical to those generated by WarpX. Specifically, the CI test does the following:Note that the lookup tables cannot be compared in binary format because the serialization routines also serialize the padding bytes, whose value is in general unspecified. These results in minor (insignificant) differences between the binary table generated by the tool and the binary table generated by WarpX