Improving the attribute system about dpsim

[JTS22]

As an initial idea, the attributes could be implemented by using two different levels of pointers. Every class that should hold attributes would have a member variable that stores a shared pointer to an attribute instance. At the same time, the underlying AttributeList manages a map that stores the attribute's ID and a shared pointer pointing to the same attribute instance. This way, every attribute of a class can be referenced both by the class' member variable as well as the entry in the AttributeList parent class.
Additionally, every attribute instance should hold a pointer to the actual value managed by this attribute. This decouples the value from the attribute's other variables like access rights or ID. Doing this should also allow for sharing a value pointer between two different attributes, which might be helpful for solving the reference issue.

When doing calculations involving the attributes, is should be possible to directly use the involved class' member variables (which would then be pointers to attributes with pointers to values). To make this as easy as possible (regarding syntax and computation effort), the dereference operator * probably has to be overloaded.

As a more advanced step, we might want to look at how the Task schedule is computed from the attributes' relations to each other. It might be possible to move the dependency information into the attributes themselves, specifying what other attributes an attribute depends on and how its value can be computed from its dependencies. While this is quite a large shift in how the scheduler manages dependencies, it might result in a simpler dependency graph and easier to understand scheduler code.

[JTS22]

dpsim/models/Include/cps/Attribute.h

Lines 256 to 267 in ad491cc

	Attribute<Real>::Ptr real() {
	Attribute<Real>::Getter get = [this]() -> Real {
	return this->getByValue().real();
	};
	Attribute<Real>::Setter set = [this](Real realPart) -> void {
	Complex copyValue = this->getByValue();
	this->set(Complex(realPart, copyValue.imag()));
	};
	return Attribute<Real>::make(nullptr, get, mFlags, shared_from_this());
	//Real *realPart = &reinterpret_cast<Real*>(mValue)[0];
	//return Attribute<Real>::make(realPart, mFlags, shared_from_this());
	}

For the new attribute system, we might also want to look at how to implement these kinds of derived attributes in a different way. Currently, these create new attributes that reference the old attribute and compute their own values using Getter and Setter functions. In this example, this behavior is inconsistent as the generated setter is never used (instead nullptr is passed into the constructor, crashing the program if the setter is ever called). Also, when statically looking at the code, it appears as if the Getter function calls itself (via this->getByValue) which would lead to an infinite loop. This code only works because this is captured in the lambda clause at a point where the Getter (and the getter flag) are not set yet. While technically correct, this is rather confusing and hard to understand when reading the code.

[m-mirz]

The main application of the getters is to process data before sending it to VILLASweb and for logging. Often we send amplitude and phase to VILLASweb instead of real and imaginary value of a complex voltage. Setters were introduced to preprocess incoming data but eventually we never used them and just made sure that the incoming data is already compliant.

Since we built the attribute system, VILLASnode introduced hooks to transform relayed data. This covers many use cases already. For logging purposes, the getters are still convenient. I could imagine that having a clean solution for the getters would be sufficient since we can use VILLASnode for other use cases.

[JTS22]

All commits related to this issue will now be collected in PR #62. In the coming days I will post an overview on the (highly experimental) changes currently contained in the PR.

[JTS22]

Here are some thoughts on the current state of the attribute system as well as an explanation of the new system currently included in PR #62 :
Currently, to my knowledge, there are three different kinds of attributes, which are all instances of the same Attribute<T> class:

• "static" attributes that own a value, which can be changed through the linked variable reference or through the get and set methods. This kind is the most common, used for example for the ID and Name of any identified object.
• "derived" attributes that do not own a value, but compute it from the value of another attribute using Getter and Setter functions. These are created by the real and imag functions on complex attributes or by the coeff function for matrix attributes. A derived attribute can have the same or a different type compared to the attribute it's derived from.
• "referencing" attributes. Currently these are not real attributes at all, but they are implemented by cloning the reference attribute and linking it to the attribute list of another object. Their purpose is to allow for one attribute to always have the same value as another attribute.

Additionally, dependencies can be defined for all of these attributes using the Scheduler and Task system. The various tasks describe which attributes need to be recalculated first when an attribute should be updated during the simulation.

The system introduced in PR #62 aims to unify these different types of attributes and dependencies, while allowing for full generality with regard to which attributes can be combined in which relationships. To do this, the new system provides two kinds of attributes which now have their own classes:

• AttributeStatic<T>: This class describes an attribute which will always own its data value and can only be changed through direct get and set functions. A static attribute can never depend on any other attribute, it can only be used as a dependency.
• AttributeDynamic<T>: While this attribute also contains a shared pointer to some data, this data should more be though of as some sort of value cache that might not accurately reflect the "true" value of the attribute. To compute this true value, a dynamic attribute can hold two lists of AttributeUpdateTasks. These tasks are invoked when either the get or set method of the dynamic attribute is called. Every task contains a list of dependency attributes as well as an actor function. This actor function receives the pointer to the dependent attribute's value cache as well as pointers to all the dependency attributes. Every actor function can therefore update the value cache (both its value as well as its pointer address) based on the values of the dependency attributes (which can be obtained via their respective get-methods. An actor function can also act in reverse, reading the cached value and updating the dependency attribute's via their set-methods. Currently, a mix of both actions would also be possible.
  When the get function is invoked on a dynamic attribute, it first calls all the actor functions registered for the get action. After these Tasks have potentially modified the cached data, a reference to this data is returned.
  For the set function, the cached value is immediately updated to the passed value. After this, all actor functions registered for the set action are called, which gives them the chance to propagate the update to any dependency attributes.

Using the dynamic attributes, the "derived" and "referencing" attributes from the old model can be recreated in a more general form:
"Derived" attributes can be represented by a dynamic attribute with one task each for the get and set action. The task for get computes the derived value from the parent attribute(s) and updates the cache, while the task for set can update the parent attribute(s) based on the new value of the derived attribute. For the real part of a complex attribute, these tasks can look something like this:

https://github.com/JTS22/dpsim/blob/98d9b471da7665144bace970bf8ba812fb23c03c/models/Include/cps/Attribute.h#L223-L230

"Referencing" attributes are implemented using a dynamic attribute with a task which updates not the cached data value, but the cache pointer address. When referencing a static attribute, it would suffice to update the cache pointer once at the beginning of the simulation. When referencing a dynamic attribute, this update has to be performed on every get operation as the data pointer of the referenced attribute could also be subject to change.

To allow for this scope of generality, the resulting attribute code gets quite complicated and template-heavy. To prevent the user from having to deal with these generic types, multiple derive functions are introduced that create new dynamic attributes from other (static or dynamic) attributes. Using C++ 20 concepts, these functions can be bound to requirements such that they are only available for the attributes with the right type. This means that, for example, the deriveReal and deriveImag functions are only available for attributes with complex type. For referencing attributes, a derive function might be introduced that correctly installs the get action for copying the data pointer, either once for static references, or on every get for dynamic references.

Lastly, I want to stress that most of these concepts currently just exist on paper and are therefore subject to change. For example, the high amount of templates might lead to an unacceptable increase in compiled code size. Or the additional function calls for management of the actor functions might cause too much of a performance overhead to actually be useful. The system might also be too general for the use case of DPsim, in which the special cases this new system allows for might never be needed.

[JTS22]

Since reworking the attributes entails reading through a lot of DPsim's current code, here are some random things I noticed that might be turned into new Issues later on:

• The MNASolver uses different attributes depending on if it computes multiple frequencies in parallel (expressed through mFrequencyParallel)- The corresponding attributes mLeftSideVectorHarm and mLeftSideVector should probably be unified
• Many of the basic components (Capacitor, Resistor, Inductance...) are (partly) derived from a base class which contains only the components main parameter (C, R, L...). Since these base classes are currently not derived from AttributeList, their attributes can not be initialized in the usual way, rather the initialization has to be moved into the subclasses.
• The clone and setParameters methods are very similar for many basic components. Maybe a clone could just clone all attributes that have a certain "parameter" flag set
• Why is Base::Ph1::Resistor::mConductance a thing? It is only used once in SP::Ph1::Resistor and is always the inverse of the resistance (as it should be?). If this is really necessary for some reason, we could make it a derived attribute.
• For some DP and SP components (e.g. {DP,SP}::Ph3::VoltageSource), the component code is nearly identical. One might consider reusing the relevant computation tasks.
• In the {DP,SP}::Ph1::NetworkInjection::daeInitialize methods, the Complex attribute "v_intf" is accessed, however, this is a complex matrix, therefore the program should crash if it ever reaches this point. Changing this to access the first element of the "v_intf"vector...
• In all NetworkInjection classes, the attribute references are set in the constructor as well as in every setParameters method. As long as setParameters does not recreate the sub-VoltageSource, the first call to setReference in the constructor should be sufficient.
• Some kind of formatter might really help make some of the calculation code more readable
• Is there a rule to determine whether a method is implemented in the header or cpp file? For simpler methods (small setParameters, getters for attributes...) it seems kind of arbitrary
  • if not header only, it should be implemented in the cpp file by default
• In DP::Ph1::PQLoadCS::initializeFromNodesAndTerminals the attribute mNomVoltage (which is a Pointer) is compared with Zero and then something is divided by the attributes value. There is likely a ->get() missing here...
  • Yes, the ->get() is missing
• The DP::Ph1::PQLoadCS sets its right_vector attribute (which is declared in MNAInterface) to reference the right_vector of its sub-component. The right_vector should not be set to dynamic, so is there a better alternative here? This also happens in {DP, EMT}::Ph1::VoltageSourceRamp.
• The Base::Ph1::PQLoad class is never used...
• The DP::Ph1::ResIndSeries has two attributes mResistance and mConductance that are either never used at all (mConductance) or only set by setParameters, but not used in any calculations (mResistance)
  • if an attribute is never used > delete, if it is used somewhere but could be removed > mark with comment
• Given the above points, a dead-code analysis for the whole project might be a good idea...
• In the DP::Ph1::RxLine, the mVoltage member variable is only read once and never written (so its always zero), while the mCurrent variable is never used at all.
  • mark with comment
• In the component classes, many member variables (especially regarding Conductance, Resistance, Capacitance, Inductance) are either never used at all (see above points) or only used once to initialize some sub-component. Many of these can probably be converted to local variables, leading to overall savings in memory usage.
  • mark with comment
• The DP::Ph1::SynchronGeneratorTrStab has an option for setting a reference for Omega and Delta (mUseOmegaRef). This can probably be implemented better using the new dynamic attributes (see comments in the code).
• The EMT::Ph3::SynchronGeneratorDQ defines a rather long list of attributes. We should check whether all of these are used within the subclasses
  • mark with comment
• The {DP,EMT}::{Ph1,Ph3}::SynchronGeneratorIdeal defines an attribute V_ref which is set to reference the voltage of the sub-component's voltage-source. However, in the component code itself, V_ref is never used. Since there is no guarantee this is ever set correctly / readable from outside code, a getter method might be better here. This also seems to apply to other components where attributes solely exist for outside code to access sub-component attributes.
  • mark with FIXME
• Every SimPowerComp<T> has a vector mSubComponents of sub-components but most classes derived from SimPowerComp have their sub-components as separate member-variables, not utilizing this list.
  • SImPowerCom list should be used, mark with FIXME
• The DP_Ph1_Transformer.cpp and EMT_Ph3_Transformer.cpp are duplicated within the Source/SP folder
  • delete duplicates
• The SP::Ph1::Load::updatePQ method references attributes P_nom and Q_nom that are never declared
  • mark with FIXME
• The SP::Ph1::PiLine defines attributes that access individual Matrix sub-components. While this can be implemented using dynamic attributes, this seems kinda overkill. Especially since these attributes only seem to be used in one example and only for logging purposes (which has derive-methods already)
  • mark with FIXME
• Some member variables from base classes (especially in Base::SynchronGenerator and Base::Ph1::Transformer) are used in some sub-classes as normal member variables and in some sub-classes as attributes. This is not doable using the new system, ever member has to be one or the other.
  • try to merge and mark
• Some of the Signal-Components (e. g. FIRFilter, Integrator, PLL, PowerControllerVSI) rely on attributes being explicitly set by outside code. While possible, this requirement should be documented or facilitated through an explicit setter method.
  • add comment
• There are components (Signal::Integrator, Signal::PLL, Signal:: PowerControllerVSI) that define referencing attributes as dependencies. The new attribute system currently has no way to express that a dynamic attribute depends on the attributes it's derived from, so this needs to be added.
  • implement setReference method
• From the various SynchronGenerator classes, some of them have no header file at all while others derive from Base::SynchronGenerator in the header, but try to call the child constructor of some nonexistant class SynchronGeneratorBase. Currently, none of these classes should compile: DP::Ph3::SynchronGeneratorDQSmpl, DP::Ph3::SynchronGeneratorVBR, DP::Ph3::SynchronGeneratorVBRStandalone, EMT::Ph3::SynchronGeneratorDQSmpl, EMT::Ph3::SynchronGeneratorDQSmplCompSource, EMT::Ph3::SynchronGeneratorVBRSmpl, EMT::Ph3::SynchronGeneratorVBRStandalone
  • will be deleted by Jan
• To be continued...

[JTS22]

Most points mentioned above are now marked in the code for PR #62 using the keywords DEPRECATED, FIXME or CHECK.

[m-mirz]

Closed by #62. Follow up on code quality issues in new issue #97.