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

add diagnostic function #537

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

add diagnostic function #537

wants to merge 4 commits into from

Conversation

jkisse
Copy link
Collaborator

@jkisse jkisse commented May 22, 2023

Add diagnostic functions to run some common checks. The function is intended to be applied if a pipeflow calculation does not converge.

Please feel free to modify and extend the function!

@jkisse jkisse added enhancement New feature or request help wanted Extra attention is needed labels May 22, 2023
@codecov
Copy link

codecov bot commented May 22, 2023

Codecov Report

Attention: Patch coverage is 1.00000% with 99 lines in your changes missing coverage. Please review.

Project coverage is 83.70%. Comparing base (314d0bf) to head (69fcb7f).

Current head 69fcb7f differs from pull request most recent head 1a459af

Please upload reports for the commit 1a459af to get more accurate results.

Files Patch % Lines
pandapipes/diagnostic.py 0.00% 79 Missing ⚠️
pandapipes/toolbox.py 4.76% 20 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #537      +/-   ##
===========================================
- Coverage    86.02%   83.70%   -2.32%     
===========================================
  Files           90       91       +1     
  Lines         6497     6180     -317     
===========================================
- Hits          5589     5173     -416     
- Misses         908     1007      +99     

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

@EPrade EPrade marked this pull request as ready for review July 15, 2024 09:04
@EPrade EPrade requested review from dlohmeier and mfranz13 July 15, 2024 09:04
logger.info(f"If sinks and sources were scaled with a factor of to "
f"{check_scaling_factor}, the pipeflow would converge.")
else:
logger.warning(f"If sinks and sources were scaled with a factor of to "
Copy link
Collaborator

Choose a reason for hiding this comment

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

See comment above.

Copy link
Contributor

Choose a reason for hiding this comment

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

You mean iterations as a function argument?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I mean the pipeflow exception that must be caught. The else-part will never be reached.

f"highest value in the net is {net.pipe.k_mm.max()}. Up to "
f"0.2 mm is a common value for old steel pipes."
f"\nRough pipes: {net.pipe.loc[net.pipe.k_mm > 0.5]}.")
net4 = net.deepcopy()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is net.deepcopy() the same as copy.deepcopy(net)?

branch_component = ["pipe", "valve", "compressor", "pump", "heat_exchanger", "circulation_pump"]
for bc in branch_component:
if hasattr(net, bc):
missing_f = np.setdiff1d(net[bc].from_junction, net.junction.index)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would prefer the following:

  1. Add some code above that checks all tables in net and makes sure that the respective model is part of the component list (an important source of errors).
  2. Iterate over all branch components and use the "from_to_junction" identifiers to check the columns



if __name__ == '__main__':
import pandapipes.networks
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think, we should rather skip main calls

@@ -615,3 +615,40 @@ def get_internal_tables_pandas(net, convert_types=True):
tbl[col] = tbl[col].astype(np.bool_)

return node_table, branch_table


def print_pf_summary(net):
Copy link
Collaborator

Choose a reason for hiding this comment

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

We would need some kwargs here (e.g. for heating grids)

Copy link
Collaborator

Choose a reason for hiding this comment

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

We could also just choose between different modes here. But for a start, we can also just add a hint that this function returns some unuseful stuff for heating grids.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants