Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issues related to attributes #97

Open
8 of 19 tasks
JTS22 opened this issue Jun 29, 2022 · 1 comment
Open
8 of 19 tasks

Fix issues related to attributes #97

JTS22 opened this issue Jun 29, 2022 · 1 comment

Comments

@JTS22
Copy link
Contributor

JTS22 commented Jun 29, 2022

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

  • In the EMT::Ph3::NetworkInjection class, 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.

  • The Base::Ph1::PQLoad class is never used. Delete it.

  • 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 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
    • solved in 0977184
  • Base::Ph1::Resistor and Base::Ph3::Resistor have a property mConductance. It is always the inverse of the resistance (as it should be?). Maybe make these derived attributes or calculate them directly in the code using them.

  • 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
    • These still exist on the current master commit
    • solved in d638c01
  • Move small methods currently implemented in header files into the corresponding cpp files when the class is not header-only

    • changed in 62710d5, 8973cc2, and bd38116
    • Exceptions: Constructors, Empty Methods, Methods of Sub-classes (as declared in the same header file, e. g. tasks), Methods with unknown template types
  • 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.

    • This happens so often it is the norm rather than the exception. For the generator classes, it is probably unreasonable to delete the entire base class, so this should either be left as is or the registration of attributes in the mAttributes list has to be changed somehow.
  • 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. However, this could lead to problems because setParameters often has side effects. Maybe make a new issue for reforming the whole parameter system?

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

  • Use a standardized formatter and check for formatting in the CI pipeline

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

  • There exists many attributes which are either never used at all or only written to but never read. Many of them are marked with FIXME or CHECK comments and can probably be deleted.

  • Perform a dead-code analysis.

  • Check, improve and document subcomponent handling #131

    • SImPowerCom list should be used, mark with FIXME
  • The SP::Ph1::Load::updatePQ method references attributes P_nom and Q_nom that are never declared

    • Is marked with FIXME
  • 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
  • Resolve FIXME, CHECK, TODO, DEPRECATED and THISISBAD comments #132

@JTS22
Copy link
Contributor Author

JTS22 commented Oct 27, 2022

Things that may be converted into new, full issues:

  • Rework the parameter system to replace the setParameters and clone methods
  • Look for and delete dead / unused code (related to unused variables, methods and classes)
  • Define and enforce a consistent code format / style

@dinkelbachjan dinkelbachjan moved this to Todo in DPsim Nov 2, 2022
dinkelbachjan added a commit that referenced this issue Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

No branches or pull requests

1 participant