[IMPROVEMENT] reduce use of shared_ptr in the core

[TonyXiang8787]

Background

In the PGM core we use std::shared_ptr in many places. Sometimes the usage is justified, but also at many places, the usage is not justified.

Shared ownership

std::shared_ptr is meant to be used for shared ownership. That is, there are multiple owners of the same resources, and we as library developer cannot determine in run-time which owner will be destroyed first. An example is shown below:

power-grid-model/power_grid_model_c/power_grid_model/include/power_grid_model/main_core/state.hpp

Lines 19 to 23 in b9a932f

	// calculation parameters
	std::shared_ptr<ComponentTopology const> comp_topo;
	std::vector<std::shared_ptr<MathModelTopology const>> math_topology;
	std::shared_ptr<TopologicalComponentToMathCoupling const> topo_comp_coup;

The ComponentTopology will be constructed after the model adds all the components. It cannot be changed. When the user copies the model (via C or Python API), the same ComponentTopology will be shared between multiple instances of MainModel. We as PGM developer cannot possibly know the ownership hierarchy (if any) of difference copies of the MainModel. Therefore, using a std::shared_ptr is justified.

Similar applies to MathModelTopology and TopologicalComponentToMathCoupling. When a model has cached math topology, and it get copied. The same topology can be shared between different instances. When one of the copied model refreshes the topology, the ownership of the old topology from that instance is released. But the old topology can still be owned by other copies, until all the copies of the model refresh the topology.

Top-down logical flow

There are many places, however, where the logical flow is strictly top-down. In these places, usage of std::shared_ptr is not justified. An example is shown below:

power-grid-model/power_grid_model_c/power_grid_model/include/power_grid_model/math_solver/iterative_pf_solver.hpp

Lines 91 to 94 in b9a932f

	std::shared_ptr<DoubleVector const> phase_shift_;
	std::shared_ptr<SparseGroupedIdxVector const> load_gens_per_bus_;
	std::shared_ptr<DenseGroupedIdxVector const> sources_per_bus_;
	std::shared_ptr<std::vector<LoadGenType> const> load_gen_type_;

The IterativePFSolver should not own the resource of phase_shift (which is part of MathModelTopology). In the logical flow, if the current the MathModelTopology is destroyed, so will IterativePFSolver also be destroyed. It does not make sense to let IterativePFSolver to own this resource. This just create extra useless abstraction. It also complicates writing the test code. Now we have to create a shared resource in the test code for the class to consume.

Proposal

We propose to remove the std::shared_ptr when shared ownership is not needed, i.e., we can know strictly that the resource will outlive the scope of the object by looking at the logical flow. We can change those members into one of following alternatives:

• Raw pointer: this is most straightforward to apply.
• Reference member: this can be interesting, but it makes the class not assignable.
• For vector like stuff, we can use a std::span.

The choices is still under discussion.

[figueroa1395]

I agree with the proposal. Here are some additional comments:

• Before using pointers, we should first resort to things like std::span or what we have now available in std::ranges and std::views.
• If the above can't be used, let's not use raw pointers either. I would argue to either use std::unique_ptr or reference members. For simplicity, I would argue to use reference members unless we are certain that we need the class to be assignable or that the resource shouldn't/can't be "shared"/"propagated", then just use a std:unique_ptr.

This way, we also express motive with the choice for each case.

Note: As a follow up cleanup-up, we should extend this search beyond std::shared_pointer to any use of std::vector or any range/view like objects.

[TonyXiang8787]

I agree with the proposal. Here are some additional comments:

• Before using pointers, we should first resort to things like std::span or what we have now available in std::ranges and std::views.
• If the above can't be used, let's not use raw pointers either. I would argue to either use std::unique_ptr or reference members. For simplicity, I would argue to use reference members unless we are certain that we need the class to be assignable or that the resource shouldn't/can't be "shared"/"propagated", then just use a std:unique_ptr.

This way, we also express motive with the choice for each case.

Note: As a follow up cleanup-up, we should extend this search beyond std::shared_pointer to any use of std::vector or any range/view like objects.

Indeed we should check if std::span of other std::views make sense. Use of std::unique_ptr would not make sense here, because we are talking about a non-owning reference to an object which is owned in the higher hierarchy.

Use a reference member causes the class to be not-assignable, which is required for many STL containers. This could be a problem. For example, a vector of MathSolver requires the MathSolver to be assignable.

Use a raw pointer should be fine in this case. It actually requires minimum code adjustment.

[figueroa1395]

Use a reference member causes the class to be not-assignable, which is required for many STL containers. This could be a problem. For example, a vector of MathSolver requires the MathSolver to be assignable.

Use a raw pointer should be fine in this case. It actually requires minimum code adjustment.

What about using std::reference_wrapper as a way to use references but keeping the classes assignable? It may need a little extra effort for the implementation, but it should work, right? Just being exhaustive with the options.