Skip to content

Al/improvements - #163

Open
AadilLatif wants to merge 6 commits into
masterfrom
al/improvements
Open

AadilLatif wants to merge 6 commits into
masterfrom
al/improvements

Conversation

@AadilLatif

Copy link
Copy Markdown
Collaborator

No description provided.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do these changes and the changes in the other example exports mean the code changes created differences in test outcomes?

@nadiavp

nadiavp commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator

Can you provide some context for this PR? This is a lot of changes at once.

@nadiavp

nadiavp commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator

Can you provide some context for this PR? This is a lot of changes at once.

Haley provided some context. I'll test with this and see if there's breaking changes that should be highlighted for users.

DampCoef = 0.5
touLoadLim = 50
%touCharge = 100
control = "scheduled"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a potentially breaking change for existing users.


return 0

def volt_var_control(self):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove some of these control options? Were they added back somewhere else?

self._controlled_element.SetParameter("kw", S_PCC.real )
self._controlled_element.SetParameter("kvar", S_PCC.imag)
self.results.append({
"Vdc" : self._pv_model.DER_model.Vdc * self._pv_model.DER_model.Vbase,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like we might be losing the controller logic here?

Point(self.__Settings['UF1 - Hz'], tMax),
Point(0, tMax)
]
UFtripRegion = Polygon([[p.y, p.x] for p in UFtripPoints])

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we losing this ability to set ride through regions? Also, it looks like the frequency calculation based on the voltage went away.

if self.__Settings['PowerMeaElem'] == 'Total':
Sin = self.__dssInstance.Circuit.TotalPower()
Pin = -sum(Sin[0:5:2])
else:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels again like we might be losing some controller logic?

1. **OpenDSS instance isolation:** should each OpenMDAO `Problem` own a dedicated OpenDSS engine instance, or should the existing process-global `opendssdirect` engine be retained behind the adapter? Dedicated instances are the safer boundary for repeated problems and future parallel execution; retaining the global engine minimizes initial changes but prevents independent simultaneous models.
2. **OpenMDAO nonlinear solver:** should the default be `NewtonSolver`, `BroydenSolver`, or `NonlinearBlockGS`? The proposed initial default is `NonlinearBlockGS` because the OpenDSS component is a black-box explicit evaluation and finite-difference Jacobians may be expensive or unreliable. A Newton/Broyden option can be exposed after convergence baselines exist.
3. **Derivatives:** should all components initially declare finite-difference partials, or should the first release declare no derivatives and use a solver that does not require them? The proposed default is no analytic derivatives and `NonlinearBlockGS`, with finite-difference metadata added only where a chosen solver requires it.
4. **Controller composition:** should a configured controller with multiple behaviors become one component with a strategy setting, or should each behavior be a separate connected component? The proposed default is one component per configured controlled element, with internal strategy dispatch that remains pure inside `compute`.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the new refactor affect whether we design new controllers to combine functionality, or have seperate controllers for each function that work together? Eg. combined PV freq and voltage ride through, or adding voltage/freq support to existing controller functionality.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants