Skip to content

Replacement of Class Referenced with std::shared_ptr#541

Draft
JanNiklasB wants to merge 38 commits into
CRPropa:masterfrom
JanNiklasB:ReferencedClassRemoval
Draft

Replacement of Class Referenced with std::shared_ptr#541
JanNiklasB wants to merge 38 commits into
CRPropa:masterfrom
JanNiklasB:ReferencedClassRemoval

Conversation

@JanNiklasB

@JanNiklasB JanNiklasB commented May 11, 2026

Copy link
Copy Markdown
Contributor

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

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

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

Copy link
Copy Markdown
Contributor Author

I am not sure why exactly the ubuntu tests are failing, I cannot reproduce the issue and the log does not give a clear answer as to why the error occurs.
I tried to reproduce the error in a docker container with the same environment as in the workflow and also on different machines, in no case did the tests fail on my side, so it might be somewhat hardware related?
I will probably be able to fix the error when I find out what exactly causes it...

@JanNiklasB

JanNiklasB commented May 11, 2026

Copy link
Copy Markdown
Contributor Author

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...

JanNiklasB and others added 2 commits May 11, 2026 20:35
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

Copy link
Copy Markdown
Contributor Author

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.

@JulienDoerner JulienDoerner added this to the 3.4 milestone May 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants