Rethink Attribute usage conventions for model parameters #540
leonardocarreras
started this conversation in
Ideas
Replies: 1 comment
|
We already have some guidelines as a starting point https://dpsim.fein-aachen.org/docs/development/attribute-usage/. We can review and make adjustments to the guideline in parallel. |
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Context
This came up in review of PR #535 (
SP_Ph1_SynchronGenerator: add reactive-power limitsQmin/Qmax), wheremQLimMax/mQLimMinwere added asAttributes following the existing convention already used throughoutSP_Ph1_SynchronGenerator. @georgii-tishenin raised two good questions during review that don't have an immediate answer and are worth tracking as a general discussion rather than resolving ad hoc per-PR:Is
Attributeexposure justified for every new model variable?Following existing convention is reasonable locally, but it's not obviously optimal as not every internal parameter needs to be part of the public, introspectable
Attributeinterface.Could some of these variables be private instead?
E.g. limiter bounds like
mQLimMax/mQLimMinmay only ever be used internally, and don't necessarily needAttributeoverhead or public exposure.Additionally, there are cases of variables that depend on each other (e.g. a physical quantity and its per-unit equivalent) that are currently stored/exposed as independent attributes. This risks them getting out of sync and adds redundant state. Fixing this likely needs a more structural approach (derived/computed attributes, or a clear single source of truth + conversion helpers) rather than a local patch.
Goal
Define a clearer convention and apply it on the codebase. This is something that needs to be harmonized beyond this single component.
Scope note
This is a discussion/design issue first. The actual refactor is likely non-trivial (touches Attribute system usage broadly) and should be broken into follow-up PRs once a convention is agreed.
References
All reactions