accumulated Carpet updates

Create issue
Issue #2203 closed
Roland Haas created an issue

Pull requests https://bitbucket.org/eschnett/carpet/pull-requests/22/rhaas-carpetupdates/diff https://bitbucket.org/cactuscode/cactus/pull-requests/53/rhaas-updates/diff https://bitbucket.org/cactuscode/cactuspugh/pull-requests/3/pugh-initialize-new-cgh-fields/diff contain accumulated updates to Carpet, Cactus and PUGH in the funhpc branches.

I have verified that the three pull requests produce bit-identical output for the qc0-mclachlan.par parfile using 5 MPI ranks and 2 threads per rank on my workstation (debian-buster, gcc 8.2).

A summary of changes:

  • add support for very large grids where 64bit integer are needed for grid indices and sizes of transfer buffers
  • fix how how physical_time_per_hour is computed
  • add functionality to align interior of grid functions to cache boundaries. This requires changes to Cactus and PUGH as well.
  • add a parameter "granularity" to make sure the interior of components is a multiple of N points in each direction
  • always align data if VECTORISE_ALIGN_FOR_CACHE is set in option list indep. of pad_to_cachelines runtime parameter
  • cleanup, reformatting, minor speedups

To compare the changes it is not advisable to look at the diff directly. It is much better to run both the current and the new version through clang-format first:

cd repos/carpet
git checkout master
git checkout -b master-formatted
clang-format -style=file -i */src/*.{cc,hh}
git commit -a -m 'Beautify code'
git checkout rhaas/carpetUpdates
git checkout -b rhaas/carpetUpdates-formatted
clang-format -style=file -i */src/*.{cc,hh}
git commit -a -m 'Beautify code'
git diff master-formatted..rhaas/carpetUpdates-formatted

which is more manageable (but still multiple thousands of lines)

Keyword: backport

Comments (8)

  1. Roland Haas reporter
    • removed comment

    These are (almost there is a single commit of mine in the Cactus pull request) Erik's changes, and I have already reviewed them and tested them on my workstation.

    I will leave this ticket open until Tuesday Oct 9 2018 and then apply all changes in the pull request unless there are objections.

  2. Roland Haas reporter
    • changed status to open
    • removed comment

    I attach the testsuite results (no failures). The tests need to be re-run after finally merging to master and before pushing to ensure that no new failures have crept in in between.

  3. Roland Haas reporter
    • marked as
    • removed comment

    I added an actual bugfix that was hidden in the openmp-tasks branch. This would cause no prolongation to happen if use_openmp = no. The bug was introduced in fe8d0d5b "CarpetLib: New option use_openmp" (2 years, 5 months ago).

    87c5dcc9 "CarpetLib: add missing non-openmp case for call_operator" should be backported to the release branch, though it seems to have not caused issues for the last 2 years (since one would normally set OMP_NUM_THREADS=1 to disable multi-threading and not change any parameter).

    Raising severity to major b/c of this (should be included in next release).

  4. Steven R. Brandt
    • removed comment

    OK, I've looked over the patches. I have a couple of minor points which I think ought to be addressed. Otherwise, if the tests pass, it should be good to apply.

  5. Roland Haas reporter
    • changed status to resolved
    • removed comment

    Thank you.

    Applied as git hash bf0c9c0d "CarpetLib: add missing non-openmp case for call_operator" of Carpet, git hash 9833bac2 "Cactus: Initialize $numlines earlier to avoid uninitialized use" of Cactus and git hash Z1df75fd "PUGH: Initialize new cGH fields “cctk_alignment” and “cctk_alignment_offset”" of CactusPUGH.

  6. Roland Haas reporter
    • removed comment

    Applied "CarpetLib: add missing non-openmp case for call_operator" as git hash 936e1ab5 "CarpetLib: add missing non-openmp case for call_operator" to the ET_2018_09 branch.

  7. Log in to comment