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

Move Fornberg coefficients calculations from WarpX to ablastr #5619

Conversation

lucafedeli88
Copy link
Member

The calculation of Fornberg stencil coefficients is rather general, and it can be shared with other projects of the BLAST family. Therefore, this PR moves the responsible functions into ablastr.

Specifically, the PR does the following:

  • 2 new files (FiniteDifference.H and FiniteDifference.cpp) are created under ablastr/math (CMakeLists.txt and Make.package accordingly)
  • the static method of the WarpX class getFornbergStencilCoefficients and the ReorderFornbergCoefficients function (originally defined in an anonymous namespace in WarpX.cpp) are moved to these new files, inside the namespace ablastr::math
  • the two methods are minimally adapted (e.g., AMREX_ALWAYS_ASSERT_WITH_MESSAGE becomes ABLASTR_ALWAYS_ASSERT_WITH_MESSAGE)
  • WarpX.cpp and SpectralKSpace.cpp (where the aforementioned functions were called) are updated

Note that with this PR SpectralKSpace.cpp does not need anymore to include the heavy WarpX.H header.

@lucafedeli88 lucafedeli88 added cleaning Clean code, improve readability component: ABLASTR components shared with other PIC codes labels Jan 30, 2025
@lucafedeli88 lucafedeli88 changed the title [WIP] Move Fornberg coefficients calculations from WarpX to ablastr Move Fornberg coefficients calculations from WarpX to ablastr Jan 30, 2025
@lucafedeli88 lucafedeli88 requested a review from EZoni January 30, 2025 15:46
@ax3l ax3l self-requested a review February 4, 2025 00:05
Copy link
Member

Choose a reason for hiding this comment

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

Kind of a tangential question, but do we currently have any CI check that compiles with GNU Make instead of CMake? I wonder if code changes like the ones in this file are actually tested automatically.

Copy link
Member

Choose a reason for hiding this comment

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

I can answer myself. Yes, I see at least one CI workflow here that uses GNU Make:

# make sure legacy build system continues to build, i.e., that we don't forget
# to add new .cpp files
build_nvcc_gnumake:
name: NVCC 11.8.0 GNUmake
runs-on: ubuntu-20.04
if: github.event.pull_request.draft == false
steps:
- uses: actions/checkout@v4
- name: install dependencies
run: |
.github/workflows/dependencies/nvcc11-8.sh
- name: CCache Cache
uses: actions/cache@v4
with:
path: ~/.cache/ccache
key: ccache-${{ github.workflow }}-${{ github.job }}-git-${{ github.sha }}
restore-keys: |
ccache-${{ github.workflow }}-${{ github.job }}-git-
- name: build WarpX
run: |
export CCACHE_COMPRESS=1
export CCACHE_COMPRESSLEVEL=10
export CCACHE_MAXSIZE=600M
ccache -z
export PATH=/usr/local/nvidia/bin:/usr/local/cuda/bin:${PATH}
export LD_LIBRARY_PATH=/usr/local/nvidia/lib:/usr/local/nvidia/lib64:/usr/local/cuda/lib64:${LD_LIBRARY_PATH}
which nvcc || echo "nvcc not in PATH!"
git clone https://github.com/AMReX-Codes/amrex.git ../amrex
cd ../amrex && git checkout --detach 78bdf0faabc4101d5333ebb421e553efcc7ec04e && cd -
make COMP=gcc QED=FALSE USE_MPI=TRUE USE_GPU=TRUE USE_OMP=FALSE USE_FFT=TRUE USE_CCACHE=TRUE -j 4
ccache -s
du -hs ~/.cache/ccache
.

@EZoni EZoni merged commit 8eab0c9 into ECP-WarpX:development Feb 11, 2025
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleaning Clean code, improve readability component: ABLASTR components shared with other PIC codes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants