Integrate LLAMA by bernhardmgruber · Pull Request #4003 · ComputationalRadiationPhysics/picongpu

bernhardmgruber · 2022-03-01T16:15:37Z

This PR integrates LLAMA for handling the memory layout of several core data structures.

Open tasks:

Integrate the LLAMA library as a subtree. For this, run the command: GIT_AUTHOR_NAME="Third Party" GIT_AUTHOR_EMAIL="picongpu@hzdr.de" git subtree pull --prefix thirdParty/llama git@github.com:alpaka-group/llama.git develop --squash
~~Pull upgrade to Boost 1.70 into separate PR~~ (done in CMake request boost 1.74.0+ #4144)
Find a better way to choose which memory layout to use in the param files
Make IO compile
Test IO works
Test performance is the same as before

After checking out this branch, you need to run git submodule update to get LLAMA.

include/picongpu/particles/Particles.tpp

BrianMarre · 2022-09-06T14:45:40Z

Also please note that PIConGPU already moved on to boost 1.74

bernhardmgruber · 2022-09-06T15:52:50Z

Also please note that PIConGPU already moved on to boost 1.74

Awesome, I updated the PR description!

BrianMarre · 2022-09-07T08:28:03Z

.gitignore

+# Visual Studio configuration and output files
+out
+/.vs


I don't think this should go into the PIConGPU dev, since PIConGPU does not especially favour VS

If this is really a big problem for you, then I can remove it. However, you ignore so many other tools' temporary files (like MacOS stuff, VS Code, even Code::Blocks and Netbeans), I don't think those additional ones would hurt.

BrianMarre · 2022-09-07T08:28:59Z

include/picongpu/CMakeLists.txt

-if(IN_SRC_POS GREATER -1)
-  message(FATAL_ERROR
-    "PIConGPU requires an out of source build. "
-    "Please remove \n"
-    "  - CMakeCache.txt\n"
-    "  - CMakeFiles/\n"
-    "and create a separate build directory. "
-    "See: INSTALL.rst")
-endif()
+#if(IN_SRC_POS GREATER -1)
+#  message(FATAL_ERROR
+#    "PIConGPU requires an out of source build. "
+#    "Please remove \n"
+#    "  - CMakeCache.txt\n"
+#    "  - CMakeFiles/\n"
+#    "and create a separate build directory. "
+#    "See: INSTALL.rst")
+#endif()


you should not change this, it is fine for your pull request, but not for the PIConGPU in general

Yeah, this should not get merged here. However, some IDEs like Visual Studio and CLion, when they splot a CMakeLists.txt, start to configure the project in a subfolder of the working copy, so they can offer better help to the developer. This is still an out-of-source build btw. and you can build picongpu fine this way as well. It is just not the intended workflow when working on a cluster.

BrianMarre · 2022-09-07T08:30:24Z

include/picongpu/fields/FieldJ.kernel


                Velocity velocity;
-                const float3_X vel = velocity(particle[momentum_], attribute::getMass(weighting, particle));
+                const float3_X vel = velocity(static_cast<float3_X>(particle[momentum_]), attribute::getMass(weighting, particle));


Is this cast really necessary? As far as I know momentum_ already should be float3_X

Yes, this is necessary when particle[momentum_] returns a proxy reference, which it can with certain LLAMA mappings.

BrianMarre · 2022-09-07T08:36:09Z

@sbastrakov or @psychocoderHPC if you a little bit of free time could you take a look, I am not yet qualified to review this ;)

BrianMarre · 2022-09-07T08:42:35Z

@bernhardmgruber you also should execute something like, after having load/installed clang format, (or an llvm packet containing it)

find share test include -iname "*.def" -o -iname "*.h" -o -iname "*.cpp" -o -iname "*.cu" -o -iname "*.hpp" -o -iname "*.tpp" -o -iname "*.kernel" -o -iname "*.loader" -o -iname "*.param" -o -iname "*.unitless" | xargs clang-format -i

on your pull request to do a clang format, and fix the formatting errors the CI is complaining about.

Note: clang-fromat version is important, PIConGPU uses 12.0.1

sbastrakov · 2022-09-07T08:56:29Z

I believe the plan is that @bernhardmgruber comes for some time to Dresden relatively soon to finish integratation of LLAMA to PIConGPU.

bernhardmgruber · 2022-11-07T15:11:17Z

I will resume on this work starting on the 28th of November. I will be at HZDR for two weeks to complete this work.

bernhardmgruber · 2022-11-30T17:36:54Z

So, I updated the branch to LLAMA develop and picongpu dev. I can compile the SPEC example again.

psychocoderHPC · 2022-12-01T10:25:40Z

could you run clang-format-12 that we can see what the CI is saying

find src/include/mallocMC example share tests include test  -iname "*.def" \
  -o -iname "*.h" -o -iname "*.cpp" -o -iname "*.cu" \
  -o -iname "*.hpp" -o -iname "*.tpp" -o -iname "*.kernel" \
  -o -iname "*.loader" -o -iname "*.param" -o -iname "*.unitless" \
  | xargs clang-format-12 -i

bernhardmgruber · 2022-12-01T10:43:50Z

could you run clang-format-12 that we can see what the CI is saying

I know, I only have clang-format-15 locally on Windows. On my Ubuntu 22.10, it is also no longer available:

bgruber@dixxi3:/mnt/c/dev/picongpu$ sudo apt install clang-format-12
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
E: Unable to locate package clang-format-12

LLAMA apt repository only has versions dev, 15 and 14 (secretly, the older ones might still be reachable, I did not check).

How about you update to clang-format-14? :)

bernhardmgruber · 2022-12-01T11:51:52Z

share/picongpu/benchmarks/SPEC/include/picongpu/param/memory.param

+    // struct ParticleFrameMapping : llama::mapping::BindAoS<false>
+    struct ParticleFrameMapping : llama::mapping::BindSoA<false>
+    // struct ParticleFrameMapping : llama::mapping::BindAoSoA<16>
+    // struct ParticleFrameMapping : llama::mapping::BindAoSoA<32>
+    // struct ParticleFrameMapping : llama::mapping::BindAoSoA<64>
+    {
+        inline static constexpr bool splitVector = false;
+    };


For:

Find a better way to choose which memory layout to use in the param files

I am using a quoted metafunction (ParticleFrameMapping) now which has an additional flag whether the Vectors should be split up into their components. That is good enough for most cases I guess. It can cover all LLAMA mappings which only depend on static/compile-time information.

psychocoderHPC · 2022-12-01T13:48:18Z

could you run clang-format-12 that we can see what the CI is saying

I know, I only have clang-format-15 locally on Windows. On my Ubuntu 22.10, it is also no longer available:
bgruber@dixxi3:/mnt/c/dev/picongpu$ sudo apt install clang-format-12
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
E: Unable to locate package clang-format-12
LLAMA apt repository only has versions dev, 15 and 14 (secretly, the older ones might still be reachable, I did not check).

How about you update to clang-format-14? :)

Unfortunately updating the formation requirement has low priority. The only nice feature which would come with clang-format-14 is the east cost const.

At the moment lifting the requirements is also hard because we have a few students currently trying to bring their extensions to PIConGPU they developed. Switching now the code style would maybe introduce too much pain for them.

psychocoderHPC · 2022-12-01T13:49:07Z

I can pull your branch and focre push the formatted version if you like.

bernhardmgruber · 2022-12-01T13:49:42Z

I can pull your branch and focre push the formatted version if you like.

We can do that once I am finished with everything else. I am actively working on this branch ATM to make the IO work as well.

include/picongpu/particles/Particles.tpp

include/pmacc/memory/boxes/CachedBox.hpp

BrianMarre · 2022-12-02T14:02:44Z

Our dev server still has a clang-format 12.0.1, in general you can install/load it using spack via the corresponding llvm pack

Also be aware that picongpu is even patch level sensitive, you need clang-format 12.0.1 specifically

PrometheusPi · 2022-12-13T15:26:43Z

@bernhardmgruber I think we still have an issue #4392

bernhardmgruber · 2023-05-04T06:44:05Z

Can I prevent the CI from running if I push changes, but keep this PR open?

BrianMarre · 2023-06-15T14:08:04Z

include/picongpu/fields/MaxwellSolver/AddCurrentDensity.kernel


 #include "picongpu/simulation_defines.hpp"

+#include "picongpu/param/memory.param"


Is this a good idea?

This is needed by VS to configure the project in open-folder mode.

Also support LLAMA frames in the IO.

chillenzer · 2024-08-09T21:53:17Z

@ComputationalRadiationPhysics/picongpu-maintainers This is not planned anymore, is it?

bernhardmgruber · 2024-08-09T23:44:18Z

A certain @psychocoderHPC fearfully admitted in the past that LLAMA may be a solution too complex to be maintained within PIConGPU. Henceforth, I expect this PR to be closed without a merge eventually.

psychocoderHPC · 2024-09-10T20:19:58Z

I will create soon a issue for the LLAMA integration to keep a link to this PR before we close it :-(
We do not have enough developers to maintain LLAMA and integrate it into PIConGPU :-(

bernhardmgruber commented Mar 1, 2022

View reviewed changes

include/picongpu/particles/Particles.tpp Outdated Show resolved Hide resolved

BrianMarre reviewed Sep 7, 2022

View reviewed changes

bernhardmgruber force-pushed the llama branch 2 times, most recently from 8582027 to d06c3cf Compare November 30, 2022 17:36

bernhardmgruber force-pushed the llama branch 3 times, most recently from 158f295 to 9317b83 Compare December 1, 2022 11:49

bernhardmgruber commented Dec 1, 2022

View reviewed changes

bernhardmgruber force-pushed the llama branch from 9317b83 to b6ef6fe Compare December 1, 2022 13:52

bernhardmgruber commented Dec 1, 2022

View reviewed changes

include/picongpu/particles/Particles.tpp Show resolved Hide resolved

psychocoderHPC reviewed Dec 1, 2022

View reviewed changes

include/pmacc/memory/boxes/CachedBox.hpp Outdated Show resolved Hide resolved

bernhardmgruber force-pushed the llama branch 3 times, most recently from 9f510dc to 3ff629c Compare December 1, 2022 18:34

bernhardmgruber force-pushed the llama branch from 3ff629c to 3acaf12 Compare December 2, 2022 15:09

bernhardmgruber force-pushed the llama branch 3 times, most recently from 9336a35 to dbc221a Compare December 20, 2022 15:34

bernhardmgruber force-pushed the llama branch from dbc221a to eae9230 Compare January 30, 2023 18:51

bernhardmgruber force-pushed the llama branch from eae9230 to 3bae2e3 Compare March 6, 2023 17:09

bernhardmgruber force-pushed the llama branch 3 times, most recently from bef18c5 to 50fc3ff Compare March 16, 2023 16:59

bernhardmgruber force-pushed the llama branch 10 times, most recently from bfa7c8c to e9f1a67 Compare May 4, 2023 06:43

bernhardmgruber marked this pull request as draft May 4, 2023 06:44

bernhardmgruber force-pushed the llama branch 3 times, most recently from 6fc8f5e to fdadfa9 Compare May 4, 2023 08:06

BrianMarre reviewed Jun 15, 2023

View reviewed changes

bernhardmgruber added 2 commits November 21, 2023 14:50

Disable error for in-source builds

5da4754

This is needed by VS to configure the project in open-folder mode.

Use llama for particle frame and shared memory DataBox layout

d008726

Also support LLAMA frames in the IO.


		#include "picongpu/simulation_defines.hpp"

		#include "picongpu/param/memory.param"

Conversation

bernhardmgruber commented Mar 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

BrianMarre commented Sep 6, 2022

Uh oh!

bernhardmgruber commented Sep 6, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BrianMarre commented Sep 7, 2022

Uh oh!

BrianMarre commented Sep 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbastrakov commented Sep 7, 2022

Uh oh!

bernhardmgruber commented Nov 7, 2022

Uh oh!

bernhardmgruber commented Nov 30, 2022

Uh oh!

psychocoderHPC commented Dec 1, 2022

Uh oh!

bernhardmgruber commented Dec 1, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

psychocoderHPC commented Dec 1, 2022

Uh oh!

psychocoderHPC commented Dec 1, 2022

Uh oh!

bernhardmgruber commented Dec 1, 2022

Uh oh!

Uh oh!

Uh oh!

BrianMarre commented Dec 2, 2022

Uh oh!

PrometheusPi commented Dec 13, 2022

Uh oh!

bernhardmgruber commented May 4, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chillenzer commented Aug 9, 2024

Uh oh!

bernhardmgruber commented Aug 9, 2024

Uh oh!

psychocoderHPC commented Sep 10, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

bernhardmgruber commented Mar 1, 2022 •

edited

Loading

BrianMarre commented Sep 7, 2022 •

edited

Loading