update binning plugin domain info docs #5660
Open
ikbuibui wants to merge 3 commits intoComputationalRadiationPhysics:devfrom
Open
update binning plugin domain info docs #5660ikbuibui wants to merge 3 commits intoComputationalRadiationPhysics:devfrom
ikbuibui wants to merge 3 commits intoComputationalRadiationPhysics:devfrom
Conversation
ikbuibui
added
documentation
chillenzer
requested changes
Mar 31, 2026
Contributor
chillenzer
left a comment
chillenzer
left a comment
There was a problem hiding this comment.
It's a great improvement, thanks! But I found some minor points for discussion before we can merge.
Comment on lines
45
to
47
| * @brief Provides knowledge of the simulation domain to the user | ||
| * Names and concept are described at | ||
| * https://github.com/ComputationalRadiationPhysics/picongpu/wiki/PIConGPU-domain-definitions |
Contributor
There was a problem hiding this comment.
This doesn't render correctly in the docs. Please either put a colon at the end of the first line or figure out how to separate them in the docs (maybe a blank line in between)?
Comment on lines
+136
to
+139
| .. code-block:: cpp | ||
|
|
||
| getParticlePosition<DomainOrigin, PositionPrecision, PositionUnits>() | ||
|
|
Contributor
There was a problem hiding this comment.
Why wouldn't those be doxygenclass:: (or rather the equivalent for functions) as well? Same for getCellIndex of course.
Comment on lines
+157
to
+187
| Domain origin | ||
| ^^^^^^^^^^^^^ | ||
|
|
||
| The reference origin for returned positions is selected with ``DomainOrigin``: | ||
|
|
||
| The available origins are: | ||
|
|
||
| .. doxygenenum:: picongpu::plugins::binning::DomainOrigin | ||
|
|
||
| Position precision | ||
| ^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| For particle positions, the precision is selected with ``PositionPrecision``. | ||
|
|
||
| The available precisions are: | ||
|
|
||
| .. doxygenenum:: picongpu::plugins::binning::PositionPrecision | ||
|
|
||
| Position units | ||
| ^^^^^^^^^^^^^^ | ||
|
|
||
| The output units for positions are selected with ``PositionUnits``. | ||
|
|
||
| The available units are: | ||
|
|
||
| .. doxygenenum:: picongpu::plugins::binning::PositionUnits | ||
|
|
||
| .. note:: | ||
|
|
||
| Using positions in SI/PIC units introduces floating point numerical errors and may be especially problematic when using the ``TOTAL`` origin with moving window, because floating-point precision decreases as the distance from the origin increases. | ||
|
|
Contributor
There was a problem hiding this comment.
You should probably say something about the return type of getParticlePosition because it is not quite obvious that/how it changes depending on which parameters you pass.
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
3 participants
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.
Improves documentation on how to use the domain info object to get positions in the binning plugin.