[FEATURE] Address the component initialization paradox

[Jerry-Jinfeng-Guo]

Status-quo

For almost all components, we have certain Comp, with default constructor taking in a CompInput as input, next to a variety of other parameters, e.g., double u_rated. In this case, my interpretation is that the args after CompInput (e.g., rated voltage) is not part of the intrinsic parameterization of the component but requirement on another fold. For instance, a Transformer component is constructed as follows: Transformer(TransformerInput const& transformer_input, double u1_rated, double u2_rated)

Why am I complaining?

In almost all test cases, we never have to construct a multitude of certain component, i.e., std::vector<Comp>. However, the container support adding multiple components (of same type) in one go by calling main_core::add_component<Comp>(*, std::vector<CompInput>::begin(), std::vector<CompInput>::end(), *). Since there were no such thing as a default value across all components (constructors), said add_component function would fail.

My argument 1

Member variables living inside Comp are intrinsic by nature, and when instantiated all parameters contributing to them should live inside CompInput.

Proposal 1

In short, we ditch the logic of distinguishing intrinsic and extrinsic parameters (in all constructors and CompInputs).

My argument 2

Assumption that all data would be passed from Python side with all the good properties should be questioned.

Proposal 2

In short, add default values to (at least part of) constructor parameters.

[mgovers]

I do agree that the current state (and mainly the syntax) makes it less readable and testable. I propose going with Proposal 2 with a slight modification:

Argument against Proposal 1

My argument against Proposal 1: we should not expect our C API users to be able to do the translation from p.u. to unit-full, especially because the units may differ on a node-by-node basis, which would be hard in the case of Proposal 1.

Argument against Proposal 2 with default values

The major downside with using default values is the fact that it's quite error-prone during future refactorings (for instance, we will not be notified at all usages when u1_rated <-> u2_rated are swapped).

Proposal 2b: using overloads

Instead of adding default values, I would make an explicit constructor overload that has unit 1. That constructor would then use the other constructor (or vice versa). See also https://github.com/PowerGridModel/power-grid-model/pull/562#discussion_r1574196049 .

The main advantage of that is that it is forbidden to e.g. only provide 1 of the arguments, making it more future-proof. In addition, any changes to the current constructor remain localized and do not affect tests for random other parts of the code because they keep using the same constructor.

Proposal 3: explicit dependent parameters

We could also make the distinction between intrinsic and extrinsic parameters even more explicit by creating a separate struct:

struct CompInput {
  // intrinsic
};
struct CompDependentInput {
  // extrinsic (with good default values)
};

class Comp {
  public:
    explicit Comp(CompInput const& intrinsics): Comp{intrinsics, CompDependentInput{}} {} // calls the one below
    explicit Comp(CompInput const& intrinsics, CompDependentInput const& extrinsics) {
      // body
    }
};

[Jerry-Jinfeng-Guo]

Argument against Proposal 1

My argument against Proposal 1: we should not expect our C API users to be able to do the translation from p.u. to unit-full, especially because the units may differ on a node-by-node basis, which would be hard in the case of Proposal 1.

I could definitely use a crush course on the exchanges over proposal 1. Let's sort it out during one of the knowledge sharing or refinement.