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

Feature: allow passing custom runpp-function to pp.diagnostics #2193

Merged
merged 5 commits into from
Mar 26, 2024

Conversation

jthurner
Copy link
Contributor

pp.diagnostic runs power flows during various tests but does not allow passing arguments to pp.runpp.

This PR adds an optional argument runpp_function to pp.diagnostic which takes a function to be used for running power flows. The argument defaults to pp.runpp, existing code is unaffected.

Arguments to pp.runpp can then be passed through pp.diagnostic by supplying a partial as the runpp_function.

@jthurner jthurner changed the title Feat/custom runpp diagnostics Feature: allow passing custom runpp-function to pp.diagnostics Dec 20, 2023
Copy link
Member

@rbolgaryn rbolgaryn left a comment

Choose a reason for hiding this comment

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

Dear @jthurner ,

thank you for this contribution!

I suggest enabling two features: 1) passing the function to be used instead of runpp and 2) passing kwargs to that function - in a way that either one is possible and you do not need to prepare the partial for the diagnostic function.

e.g.

diagnostic(net, init="results")
diagnostic(net, runpp_function=pp.rundcpp)
diagnostic(net, runpp_function=pp.rundcpp, trafo_loading="power")

Also, I suggest renaming the parameter to "run" to be consistent with the pp.run_timeseries where you can pass the runpp function as the "run" parameter.

@jthurner
Copy link
Contributor Author

Dear Roman,
the signature should now be consistent with pp.run_timeseries, please check if these changes are what you had in mind.

@jthurner jthurner requested a review from rbolgaryn January 11, 2024 09:27
Copy link

codecov bot commented Jan 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.44%. Comparing base (f7d0f1c) to head (21fee72).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #2193   +/-   ##
========================================
  Coverage    79.43%   79.44%           
========================================
  Files          255      255           
  Lines        27947    27952    +5     
========================================
+ Hits         22201    22206    +5     
  Misses        5746     5746           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rbolgaryn
Copy link
Member

Dear @jthurner ,

please update the branch (merge develop) and add a mention of this change to CHANGELOG.

@rbolgaryn
Copy link
Member

for some reason I cannot update the branch (usually there is a button "Update branch" but not in this case)

@jthurner jthurner force-pushed the feat/custom_runpp_diagnostics branch from 58f1193 to 2b7b1a0 Compare March 26, 2024 09:05
@jthurner
Copy link
Contributor Author

Merged develop and updated CHANGELOG.

for some reason I cannot update the branch (usually there is a button "Update branch" but not in this case)

Because the fork is under an organization account, I can not grant "Allow edits from maintainers" on the PR (see e.g.
orgs/community#5634). Pretty annoying, we might have to open future PRs from personal branches instead.

@rbolgaryn rbolgaryn merged commit 0ad433d into e2nIEE:develop Mar 26, 2024
19 checks passed
@jthurner jthurner deleted the feat/custom_runpp_diagnostics branch December 3, 2024 12:16
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.

2 participants