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

Docs: improve "Workflows" section for developers #5525

Open
wants to merge 7 commits into
base: development
Choose a base branch
from

Conversation

EZoni
Copy link
Member

@EZoni EZoni commented Dec 20, 2024

Suggestion for reviewers:
Compare the preview of the new documentation available from the CI check docs/readthedocs.org:warpx with the existing documentation available online.

Improve the Workflows documentation for developers, so far only the following sections:

  • "How to test the code"
  • "How to run the clang-tidy linter"

To-do:

  • Incorporate minimal checksum documentation in one single test section
  • Add step-by-step guide on how to add new tests (enumerated list of steps)

@EZoni EZoni added component: documentation Docs, readme and manual component: tests Tests and CI labels Dec 20, 2024
@EZoni EZoni changed the title [WIP] Improve "Workflows" docs for developers, simplify addition of new CI tests [WIP] Docs: improve "Workflows" section for developers Dec 20, 2024
Comment on lines -233 to -270

Useful tool for plotfile comparison: ``fcompare``
-------------------------------------------------

AMReX provides ``fcompare``, an executable that takes two ``plotfiles`` as input and returns the absolute and relative difference for each field between these two plotfiles. For some changes in the code, it is very convenient to run the same input file with an old and your current version, and ``fcompare`` the plotfiles at the same iteration. To use it:

.. code-block:: sh

# Compile the executable
cd <path to AMReX>/Tools/Plotfile/ # This may change
make -j 8
# Run the executable to compare old and new versions
<path to AMReX>/Tools/Plotfile/fcompare.gnu.ex old/plt00200 new/plt00200

which should return something like

.. code-block:: sh

variable name absolute error relative error
(||A - B||) (||A - B||/||A||)
----------------------------------------------------------------------------
level = 0
jx 1.044455105e+11 1.021651316
jy 4.08631977e+16 7.734299273
jz 1.877301764e+14 1.073458933
Ex 4.196315448e+10 1.253551615
Ey 3.330698083e+12 6.436470137
Ez 2.598167798e+10 0.6804387128
Bx 273.8687473 2.340209782
By 152.3911863 1.10952567
Bz 37.43212767 2.1977289
part_per_cell 15 0.9375
Ex_fp 4.196315448e+10 1.253551615
Ey_fp 3.330698083e+12 6.436470137
Ez_fp 2.598167798e+10 0.6804387128
Bx_fp 273.8687473 2.340209782
By_fp 152.3911863 1.10952567
Bz_fp 37.43212767 2.1977289
Copy link
Member Author

Choose a reason for hiding this comment

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

Do developers really use this when adding automated tests? If it's a tool that isn't used much, I would rather remove this section to keep the documentation as concise as possible.

Copy link
Member

Choose a reason for hiding this comment

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

I do use fcompare occasionally. But I agree that this level of description is not needed. Maybe some place here just mention that that tool is available.

@ax3l ax3l self-requested a review January 2, 2025 23:07
@EZoni EZoni marked this pull request as ready for review January 9, 2025 22:12
@EZoni EZoni requested a review from aeriforme January 9, 2025 22:12
@EZoni EZoni changed the title [WIP] Docs: improve "Workflows" section for developers Docs: improve "Workflows" section for developers Jan 9, 2025
Copy link
Member Author

Choose a reason for hiding this comment

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

FYI, this was actually renamed from run_clang_tidy_locally.rst and modified later on, but the number of changes is large enough to make GitHub display it as a new file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: documentation Docs, readme and manual component: tests Tests and CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants