Many bug and warning fixes by JanNiklasB · Pull Request #537 · CRPropa/CRPropa3

JanNiklasB · 2026-04-01T00:12:00Z

Hey,

with this PR I want to introduce the new CXX standard 23 to CRPropa, it enables us to use more modern code in future additions which should make it easier to contribute.
Furthermore CXX23 introduces the new deducing this which enables us to use CRTP (Curious Reoccurring Template Patterns) instead of vtables while keeping the fundamental structures of CRPropa, this is important for an eventual GPU extension.

I also tried to tackle many warnings that arose in the past, in the process of that I discovered some major bugs which I also fixed here (see comments in code for more details):

type instead of to_type in Variant.cpp
missing return in Variant.cpp
deprecated / missing ENDDO statements in sophia_interface.f
broken doxygen commands
depricated sprintf replaced with snprintf
missing include path in FindGooglePerfTools.cmake
missing brackets in Random.cpp
not initialized constScaleBendover in preconstructor
extends never set in PeriodicMagneticField::setExtends
never set Bphi in CMZField.cpp if r<=r1
issue between clang and gcc when formatting uint64_t, now using macro "%10" PRIu64 "\t" which set it automatically
added more modern build tool ninja to workflows

This caused INT_CASE to be always false

JanNiklasB · 2026-04-01T00:15:28Z

include/crpropa/AssocVector.h

        {
-            typedef std::pair<typename C::first_argument_type, Value>
+            typedef K first_argument_type;
+            typedef std::pair<first_argument_type, Value>


first_argument is removed in CXX23 so we need an additional template argument, the rest of the code is adjusted, user code is not affected as far as I see

JanNiklasB · 2026-04-01T00:16:21Z

include/crpropa/AssocVector.h

-        typedef typename A::reference reference;
-        typedef typename A::const_reference const_reference;
+        typedef typename A::value_type& reference;
+        typedef const typename A::value_type* const_reference;


reference and const_reference are removed in CXX23, value_type is equivalent

JanNiklasB · 2026-04-01T00:17:25Z

include/crpropa/AssocVector.h

        class value_compare
-            : public std::binary_function<value_type, value_type, bool>
-            , private key_compare
+            : private key_compare


binary_function is not needed in newer CXX versions, its functionality is now done automatically and therefore it was removed

JanNiklasB · 2026-04-01T00:18:31Z

libs/sophia/sophia_interface.f

ENDDO is the more modern approach, the old one also works but throws warnings

JanNiklasB · 2026-04-01T00:19:29Z

src/magneticField/turbulentField/PlaneWaveTurbulence.cpp

 #ifdef ENABLE_FAST_WAVES
 #include <immintrin.h>
-#include <memory.h>
+#include <memory>


<memory> is more general, we had vocally reported issues in the past and <memory> seemed to fix them

JanNiklasB · 2026-04-01T00:20:06Z

src/magneticField/CMZField.cpp

    }
    else{
-        - B1*exp(-z*z/Hc/Hc)*R/r1*(3*r/r1- 2*r*r/r1*r1);
+        Bphi = - B1*exp(-z*z/Hc/Hc)*R/r1*(3*r/r1- 2*r*r/r1*r1);


was forgotten

JanNiklasB · 2026-04-01T00:20:26Z

src/magneticField/MagneticField.cpp

 }

-void PeriodicMagneticField::setExtends(const Vector3d &origin) {
+void PeriodicMagneticField::setExtends(const Vector3d &extends) {


propably a typo

JanNiklasB · 2026-04-01T00:21:57Z

src/module/TextOutput.cpp


 	if (fields.test(SerialNumberColumn))
-		p += std::sprintf(buffer + p, "%10lu\t",
+		p += std::snprintf(buffer + p, buffersize - p, "%10" PRIu64 "\t",


"%10" PRIu64 "\t" is the same as "%10lu\t" in case of gcc and the same as "%10llu\t" in case of clang (that was a problem before)

JanNiklasB · 2026-04-01T00:22:27Z

src/Random.cpp

 	int64_t b = randInt();
-	return (b + a << 32);
+	// a is shifted to the left to create a proper 64 bit integer
+	return (b + (a << 32));


same as the old one, but does not throw a warning

JanNiklasB · 2026-04-01T00:23:03Z

src/Random.cpp

-		*p = twist(p[M - N], p[0], p[1]);
-	*p = twist(p[M - N], p[0], state[0]);
+		*p = twist(p[+M - N], p[0], p[1]);
+	*p = twist(p[+M - N], p[0], state[0]);


enum-enum-conversion was removed, the + fixes that

JanNiklasB · 2026-04-01T00:23:19Z

src/Variant.cpp

 			std::string s;
 			v.push_back(s);
 		}
+		return Variant(v);


was missing for some reason

JanNiklasB · 2026-04-01T00:23:30Z

src/Variant.cpp

 #define INT_FUNCTION(to_type, fun, to)                                                                                 \
 	to Variant::fun() const {                                                                                          \
-		switch (type) {                                                                                                \
+		switch (to_type) {                                                                                                \


JanNiklasB · 2026-04-01T00:25:04Z

CMakeLists.txt


-set(CMAKE_CXX_STANDARD 11)
+set(CMAKE_CXX_STANDARD 23)
+set(CMAKE_CXX_STANDARD_REQUIRED True)


Throws an error during configure step if the cmake standard is not met instead of some undefined behavior during build time, that should help the user to determine potentially outdated CXX standards

rafaelab · 2026-04-01T07:22:51Z

Hi @JanNiklasB

Thanks for the PR.

AssocVector is actually not needed, and neither is ref_ptr. A good redesign (e.g., CRPropa 4) would get rid of them and use the smart pointers from C++. The same is true for variants, since newer C++ standards already handle them quite well.
Ninja is NOT available by default in some systems, so this might be adding more complexity to CRPropa and making it harder for some people to install, especially in clusters.
You say you introduced C++23, but I don't see this strict requirement in cmakelists. If you say C++23 thinking of CRTP only, CRTP is a pattern applicable to other C++ standards. If C++23 it becomes a strict requirement, it might be problematic for older computing clusters. Therefore, while I'm totally in favour of updating CRPropa and use ≥ C++20, I have mixed feelings about doing that now instead of investing in a redesign.

JanNiklasB · 2026-04-01T10:24:04Z

Hey,
thanks for the quick answer.

I would also be in favor of a redesign regarding AssocVector and ref_ptr for CRPropa4. But it feels like CRPropa 4 is still not that close, or is there something planned for the close future?
True, Ninja is not a system default, but it is easily installable over pip or local package managers like conda.
Since it speeds up the compilation process of our workflows and is as easily available as numpy which we already require I thought it would be better to use it in the workflows, but I could of course reverse that change.
I was mainly thinking about "deducing this" or Explicit object member functions which where introduced in CXX23.
I thought setting set(CMAKE_CXX_STANDARD 23) would be enough to make the requirement, we certainly can use the new features with this flag.
I'm only working on newer clusters, but it seems to me that when working on clusters it is common to use package managers like conda, which normally enable the possibility of higher CXX standards despite an older system.
Or would that be a hardware issue?
However, I would be ok to reduce the CXX standard to 20 for now, as long as we get rid of the deprications.

lukasmerten · 2026-04-01T10:56:02Z

Hi @JanNiklasB

I agree with @rafaelab that we should be careful with backward compatibility on older compute clusters. At a minimum we should include a warning in the next v3.3 release notes that we will increase the minimum required version in the future. In this way, people will have a relatively recent official release as a fall back option, when their cluster does not support cxx20/23.

How far CRPropa v4 is in the future depends in the end on us as the developers and can/should probably discussed in the next in person meeting.

For ninja, I guess that people will not have issues to install that if they manage to install numpy. However, I suggest to wait with that until after the v3.3 release.

JanNiklasB · 2026-04-01T11:16:50Z

Ok, I still would like to at least get rid of the warnings and the few typo caused bugs, should I modify this PR or make a new one without the changes to CMAKE_CXX_STANDARD and workflows?

lukasmerten · 2026-04-01T11:34:35Z

Fair enough. Fixing the typos and small bugs sounds good to me.
Do what ever is easier for you: updating this PR or creating a new one does not matter to me.

JanNiklasB · 2026-04-01T12:07:23Z

I reversed the CXX23 requirement and ninja from the worklows, and changed the title accordingly.

JanNiklasB · 2026-04-01T12:34:28Z

I am not sure why the ubuntu64 / 64x test failed, it happened in testInteraction in line 533 where the secondary size was equal to zero, but since this was no problem in the other tests and there is a probability* (even though it is small) that no secondary is generated in this test, I think it is fine.

*:
ElasticScattering.cpp line 104-108:

// check for interaction
Random &random = Random::instance();
double randDist = -log(random.rand()) / rate;
if (step < randDist)
	return;

JanNiklasB added 20 commits March 31, 2026 18:40

Changed to current CXX23 standard

98c9f26

binary_function not needed anymore

891589d

fixed deprecated enum-enum-conversion

f94a0ab

removed lower cmake standard in testing_OSX.yml

3a3d9df

replaced deprecated ::first_argument_type

c373f3e

added missing return in Variant.cpp

9771e84

Fixed deprecation fortran warnings in sophia

fbd0eee

repaired doxygen comments

483ad6d

changed sprintf to snprintf (safer and not deprecated)

ff0433a

added newer include path to possible search paths

e276924

changed to the more general <memory>

1cbffcb

added brackets to avoid warning and comment to clarify

9501169

constScaleBendover not initialized at that time

afce3c3

setExtends did nothing

6f60299

expression was never used...

afb9dca

fixed very crucial typo (type -> to_type)

36f351d

This caused INT_CASE to be always false

fixed wrong formatting

ca42395

corrected buffersize with subsequent calls

92b94ac

fixed issue with "%ull" when formatting uint64_t between clang and gcc

033af5e

added ninja to workflows

ad93a46

JanNiklasB commented Apr 1, 2026

View reviewed changes

src/Variant.cpp

std::string s;

v.push_back(s);

}

return Variant(v);

Copy link
Copy Markdown

Contributor Author

JanNiklasB Apr 1, 2026

Choose a reason for hiding this comment

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

was missing for some reason

JanNiklasB commented Apr 1, 2026

View reviewed changes

JanNiklasB added 3 commits April 1, 2026 13:42

removed ninja from worklows

9ba0a15

set CMAKE_CXX_STANDARD back to 11 for now

ac82689

reversed cmake --build . back to make, kept ctest --output-on-failure

763e934

JanNiklasB changed the title ~~New CXX standard 23 plus many bug and warning fixes~~ Many bug and warning fixes Apr 1, 2026

Conversation

JanNiklasB commented Apr 1, 2026

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!

JanNiklasB Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

rafaelab commented Apr 1, 2026

Uh oh!

JanNiklasB commented Apr 1, 2026

Uh oh!

lukasmerten commented Apr 1, 2026

Uh oh!

JanNiklasB commented Apr 1, 2026

Uh oh!

lukasmerten commented Apr 1, 2026

Uh oh!

JanNiklasB commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JanNiklasB commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

JanNiklasB Apr 1, 2026 •

edited

Loading

JanNiklasB commented Apr 1, 2026 •

edited

Loading

JanNiklasB commented Apr 1, 2026 •

edited

Loading