Elements: Consistent Defaults#1449
Open
ax3l wants to merge 4 commits into
Open
Conversation
ax3l
commented
May 4, 2026
ax3l
commented
May 4, 2026
ax3l
commented
May 4, 2026
ax3l
commented
May 4, 2026
In our current workflow, we have to repeat ourselves quite a bit when it comes to defaults of element parameters: - C++ constructor: purely informational, not used anywhere unless someone uses ImpactX as C++ library and does not pass explicit values (internally, we do pass explicit values in init) - InitElements: the ParmParse/inputs defaults - elements.cpp: the Python defaults - parameters.rst: the ParmParse/inputs docs - python.rst: the Python docs This PR simplifies this for the first three to a single, authorative source: the C++ implementation of the element.
808be0f to
bdafe10
Compare
bdafe10 to
3388252
Compare
3388252 to
fdd3ffd
Compare
ax3l
commented
May 4, 2026
Open
ax3l
commented
May 4, 2026
ax3l
commented
May 4, 2026
Old C++ default was 10, too (init elements). C++ docs were off to 5. Python & its docs where off to 5.
cemitch99
reviewed
May 7, 2026
Member
cemitch99
left a comment
There was a problem hiding this comment.
I'm taking my time a bit with this one. I have a slight concern that this increases the complexity of the element code by introducing a large number of DEFAULT_ parameters, most of which are zero. Otherwise, I agree it is a good way to ensure consistency.
Member
Author
|
Yeah, I found the consistency to be worth the slight verbosity. Let me know where you arrive at @cemitch99 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In our current workflow, we have to repeat ourselves quite a bit when it comes to defaults of element parameters:
This PR simplifies this for the first three to a single, authoritative source: the C++ implementation of the element.
PR #699 will then also unify the
python.rstfile into the same authorative source, leaving only theparameters.rstto be manually (or LLM-y) kept right.Result, less repetition: 5x same value -> 2x same value
DRY programming like this is desirable where possible (not a hard rule*), since repetition leads quickly inconsistencies over time (e.g., #1445).
(*Programming everything DRY can also lead to hard-to-extend, rigid code, so we strike a balance.)
Changed Defaults
Same as #1445, the
ExactCFbend,ExactMultipole,ExactQuadnow use 10 mapsteps as the default.Old C++ default was 10, too (init elements). But: