Replacement of Class Referenced with std::shared_ptr by JanNiklasB · Pull Request #541 · CRPropa/CRPropa3

JanNiklasB · 2026-05-11T11:45:19Z

Hi,

as discussed in the workshop I want to replace the Referenced class with std::shared_ptr.
Referenced was used to reference count in crpropa versions that required C++ standards lower then C++11, however since C++11 the std::shared_ptr template class was introduced as a better manageable smart pointer provided by C++.

To properly use `std::shared_ptr` instead of `Referenced` some other general modifications needed to be done:

All functions accepting a raw pointer needed to be deleted. This is because otherwise the ref_ptr might be converted into a raw pointer and then back into a ref_ptr which would cause the reference count of the std::shared_ptr to not be synced anymore and therefore might cause the unintended deletion of pointers.
All stack objects now need to be handed over directly:

// correct:
{
Candidate C;
ref_ptr<Candidate> cand_ptr(C);
}
// incorrect:
{
Candidate C;
ref_ptr<Candidate> cand_ptr(&C);  // will throw double free error
}

The plugin template changed due to the now required use of ref_ptr for the process functions. This breaks most plugins, but I provided an easy fix which can just be copy pasted in the README.md

The following will work the same as before:

Handing over heap allocated pointer:

ref_ptr<Candidate> c = new Candidate();

Initializing or adding modules directly over heap allocations (are converted to std::shared_ptr when converted to ref_ptr in function call)

ModuleList SIM;
SIM.add(new SimplePropagation());

usage of python

For that the SWIG header needed some adjustments so the pointer ownership is completely on the C++ side

Some other changes include:

Whitespace changes to consistently have tabs everywhere (as required by Contribute.md)
Replacement of AssocVec by std::unordered_map
Replacement of Clock by std::chrono::high_resolution_clock
New TEST(Candidate, clone) and TEST(Candidate, cloneRecursive)
New Plugin tests that test if it is still compatible with older crpropa versions
.vscode added to .gitignore, this is a folder that is automatically generated by Visual Studio Code and should never be committed
Version tag and configurable Version.h.in so plugins can easily check which code version they building against.

I marked this PR as Draft for now (currently CRPropa3.2.1) so it is clear that we only intend to add this after we release CRPropa3.3.0 . Furthermore, with the upcoming #535 and the heavy nuclei PR this PR would only delay the 3.3 release more, so considering that and the required changes to this PR when the mentioned PRs are merged I think a draft PR is the best choice.

(intern calls need to be adjusted)

ref_ptr now acts like a normal smart pointer and is able to hold a reference by using ref_ptr(T& obj)

comment previously commented test for now, search fix later

JanNiklasB · 2026-05-11T18:00:07Z

Turns out the issue was caused by the TEST(SynchrotronRadiation, PhotonEnergy) function in testInteraction.cpp , nothing went wrong internally but the memory required by the function was greater then the available amount in the runner (>2GiB), so limiting the samples solved the issue...

The following changes are included: - Optional python (now not required to build crpropa) - Limit in secondary samples in tests to limit necessary memory - Some performance adjustments

JanNiklasB · 2026-05-12T12:47:43Z

I discussed the modification of SynchrotronRadiation.PhotonEnergy with @JulienDoerner and multiplied weights during the calculation of the average energy.

I also ran the test with the 1000 max sample limit 1000 times on our cluster to check if I encounter any fails (the test takes about 1ms) , and so far I did not encounter any.

…dClassRemoval

…moval

…' into ReferencedClassRemoval

…_VERSION tag for plugins

…t when writing plugins

…moval

rafaelab · 2026-07-13T15:16:17Z

Hi @JanNiklasB,

Thanks for the PR.

Out of curiosity: does this affect the performance?
Have you compared the native shared_ptr with CRPropa's handwritten ref_ptr for a couple of representative cases?
My intuition tells me the native C++ smart pointer should be faster, but who knows...

I haven't looked into the code in detail (nor do I think I will have time this week, so someone else might want to test it), but something I've noticed is that there seems to be a lot of outdated code in the commit (like deleting Eigen, lenses, etc), probably because you started working on this before 3.3.

JanNiklasB · 2026-07-13T15:20:40Z

Yes, there is a lot of outdated code, I am currently working on migrating the new changes to this PR.
I will also try to add some perfomance plots that compare this PR and the last release when I am done with the cleanup.

…ts.txt

…libcrpropa.so

- Merge CRPropa#569 - Workaround by manually specifying `LD_LIBRARY_PATH` and `CRPropa_INSTALL_PREFIX`

JanNiklasB · 2026-07-13T18:16:25Z

This PR is now ready for review!
I will add the performance tests probably tomorrow.

JulienDoerner

Hey @JanNiklasB, I think this PR already looks quite good. I have only a couple of minor comments.

JulienDoerner · 2026-07-14T06:23:26Z

-	       "Replace it with a more appropriate turbulent field model instance.";
-	HelicalGridTurbulence::initTurbulence(grid, Brms, lMin, lMax, alpha, seed,
-	                                      H);
+		<< "initTurbulence is deprecated and will be removed in the future. "


should we remove this one? It is already long time deprecated?

oh, I am confused how that is there, I do not remember changing this (it has nothing to do with this PR and must come from an older version or something).

Yes it is already older and you just applied the style convention. But I noticed it now, and we should remove it.

We should remove it in another PR, I will set this line as it is in the master currently for now.

JulienDoerner · 2026-07-14T06:32:54Z

 	void setPhotonField(ref_ptr<PhotonField> photonField, bool superheavy = false);

-	// decide if secondary photons are added to the simulation
+	/// decide if secondary photons are added to the simulation


JulienDoerner · 2026-07-14T06:36:38Z


 	/** Merge other maps, add pdfs */
-	void merge(const EmissionMap *other);
+	void merge(ref_ptr<const EmissionMap> other);


Are those EmissionMap Pointers available in swig?

JulienDoerner · 2026-07-14T06:40:11Z

 	 * before continuing with the primary candidate if secondariesFirst is set to true
 	 */
-	void run(SourceInterface* source, size_t count, bool recursive = true, bool secondariesFirst = true, bool waitForSecondaries = true);
+	void run(ref_ptr<SourceInterface> source, size_t count, bool recursive = true, bool secondariesFirst = false, bool waitForSecondaries = true);


Why are you changing the default behavior of the secondariesFirst flag?

no reason, this was by mistake during the last merge

JulienDoerner · 2026-07-14T08:08:22Z

+TEST(Candidate, clone){
+	Candidate c(
+		nucleusId(1, 1),  // id
+		1*keV,  // energy


Although this test will work as intended, I would suggest using relativistic energies here (e.g. 1 EeV). At the moment, an energy of $E = 1 \ \mathrm{keV}$ does not reflect CRPropa's application.

JulienDoerner · 2026-07-14T08:09:11Z

+TEST(Candidate, cloneRecursive){
+	Candidate c(
+		nucleusId(1, 1),  // id
+		1*keV,  // energy


JulienDoerner · 2026-07-14T08:36:35Z

+      - name: Checkout repository
+        uses: actions/checkout@v6
+        with:
+          ref: "3.3"


If I understand the logic correctly, this will test the old CRPropa version 3.3 with the plugin template at this point, not the new plugin with your compatibility check.

yes, somehow I did not think about this, but yes the plugin template is also set to 3.3 .
Pulling the 3.3 repository manually and installing it should fix it.

JanNiklasB · 2026-07-14T20:00:44Z

While doing the performance tests I noticed how slow std::shared_ptr actually is. Using ref_ptr like I use it in this PR would slow down the code quite a lot.

This is mainly caused by the excessive use of ref_ptr I introduced, which makes our code very memory safe (and thread safe I think) but at the cost of overall slower code, which is unacceptable.

Here are some plots that compare the current PR with the last release, there you can see how much slower the code would get (the code which I used to create these can be found in this codespace.

I want to see if I can find a workaround where we have the same or better performance as with the Referenced class since I really want to get rid of it. If I do not find anything I will close this PR.

JanNiklasB · 2026-07-15T12:35:35Z

I went through possible Ideas together with @JulienDoerner and thought about what I could do to achieve similar performance to the current release:

Keep the current version of Referenced.h and change all performance critical functions back to raw pointer handling. For that I would also need to change some internal handling of ref_ptr like in ModuleList.cpp where modules are de-referenced in each process step.
Rewrite the std::shared_ptr so that it is faster (remove everything that we do not need)
Keep old Referenced.h

Option 1 and 2 would require massive amounts of work just to achieve similar performance, since after all, it is always faster to use a raw pointer then any smart pointer since the smart pointer always needs to do at least additional checks.
So for now I would just keep the old Referenced.h and the Referenced class.

Besides from just removing the class Referenced dependencies and enabling a ref_ptr from every object instead of just objects that inherit class Referenced, this PR introduced some other related changes that I would like to still merge:

Removal of AssocVector.h and Clock.h
Double free error message bug fix
Performance improvements through noexcept flags in ref_ptr

I will make a PR in the next few days for each of those points.

@JulienDoerner and I think it would be best to set this PR as a draft again until the next CRPropa call instead of just closing it.

As a conclusion, I think this PR shows us that the std::shared_ptr is not a good enough replacement for class Referenced. However, if another smart pointer that is as fast as class Referenced will be developed, std::shared_ptr can just be replaced by it.

JanNiklasB added 30 commits April 3, 2026 20:13

removed workflows for this complete seperate branch

718d665

Merge remote-tracking branch 'origin/CPP23CRPropa' into CRRW_CRPropa

efb8c8b

replaced AssocVector..h with unordered_map

cee805c

replaced Clock with chrono

17bfae7

Replaced Referenced class with shared_ptr

63e1a05

(intern calls need to be adjusted)

wrapped bare new calls in shared_ptr

50e27cb

reworked memory in tests

7de1e3a

replaced pointer with ref_ptr

7075b4f

bugfix

fed3880

added virtual destructors for base classes

6fff4e0

added overloads for raw pointer to every function

9871af2

added operator== to ref_ptr

52e2b2a

bugfix

d248a39

removed _ptr from ref_ptr

a0c3da6

ref_ptr now acts like a normal smart pointer and is able to hold a reference by using ref_ptr(T& obj)

reverted tests and fixed tabs

7c6f7eb

changed occurences of stack pointer to references

0bdfe9b

added _raw_ptr only for stack pointer reference

30e7021

small bugfix, still encountering double frees

9cb2cad

added get_shared functions and changed 0 to NULL

a670578

added clone tests

1a69f4e

removed .get() functions where they don't belong

841c9cf

fixed emission call behaviour

118840c

reimplemented workflows

6fd8e84

fixed wrong indents

61bf8b5

removed raw pointer overloads

4f5d0bb

added --output-on-failure to ctests

fbc9ed7

switched operator== to operator T*

bd67452

added shared_ptr to some classes, most derived still missing

4c16b51

attempted fix for Vector3 python issue

f50b43b

comment previously commented test for now, search fix later

Merge remote-tracking branch 'origin/master' into ReferencedClassRemoval

dfa4c92

JanNiklasB and others added 2 commits May 11, 2026 20:35

Resolved issue encountered in the ubuntu workflows

74a35dc

The following changes are included: - Optional python (now not required to build crpropa) - Limit in secondary samples in tests to limit necessary memory - Some performance adjustments

Added weights to calculation of avg energy

a6b10a0

JulienDoerner added this to the 3.4 milestone May 26, 2026

JanNiklasB added 10 commits May 26, 2026 18:15

Merge remote-tracking branch 'CRPropa/CRPropa3/master' into Reference…

c49f021

…dClassRemoval

Merge remote-tracking branch 'CRPropa3/master' into ReferencedClassRe…

39927d2

…moval

Merge remote-tracking branch 'GDMarco/PositionDependentPhotonFieldOpt…

1b6880f

…' into ReferencedClassRemoval

changed operator = argument to nullptr_t

cc61f33

added instructions for plugin compability with older CRPropa Versions

44b314e

replaced Version.h by configurable Version.h.in to define new CRPROPA…

99849b2

…_VERSION tag for plugins

included crprop/Version.h in module so user have immidiate acces to i…

273a0d4

…t when writing plugins

Merge remote-tracking branch 'CRPropa3/master' into ReferencedClassRe…

622295a

…moval

updated plugin to now removed Referenced class

fa3bd7c

Added changes to CHANGELOG.md

d03144c

JanNiklasB and others added 6 commits July 13, 2026 17:30

reverted unintended changes

8f2694e

reversed meaningless changes in tests and corrected error in CMakeLis…

ca30769

…ts.txt

added venv install to old-crpropa tests

e85204c

fix not working tests by activating python venv in every step

bf8157c

fixed plugin not finding crpropa in venv and _crpropa.so not finding …

a95e7ca

…libcrpropa.so

fixing plugin tests (#5)

1825f3d

- Merge CRPropa#569 - Workaround by manually specifying `LD_LIBRARY_PATH` and `CRPropa_INSTALL_PREFIX`

JanNiklasB marked this pull request as ready for review July 13, 2026 18:16

JulienDoerner reviewed Jul 14, 2026

View reviewed changes

JanNiklasB marked this pull request as draft July 15, 2026 12:35

JanNiklasB closed this Sep 25, 2026

Conversation

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

Uh oh!

To properly use std::shared_ptr instead of Referenced some other general modifications needed to be done:

The following will work the same as before:

Some other changes include:

Uh oh!

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

Uh oh!

Uh oh!

JanNiklasB commented May 12, 2026

Uh oh!

rafaelab commented Jul 13, 2026

Uh oh!

JanNiklasB commented Jul 13, 2026

Uh oh!

JanNiklasB commented Jul 13, 2026

Uh oh!

JulienDoerner left a comment

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!

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 commented Jul 14, 2026

Uh oh!

JanNiklasB commented Jul 15, 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 commented May 11, 2026 •

edited

Loading

To properly use `std::shared_ptr` instead of `Referenced` some other general modifications needed to be done:

JanNiklasB commented May 11, 2026 •

edited

Loading

JanNiklasB commented Jul 15, 2026 •

edited

Loading