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

Issue10 bidirectional #12

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Issue10 bidirectional #12

wants to merge 7 commits into from

Conversation

TBeckhoelter
Copy link
Contributor

Added new node_type "supply_heating_cooling" in methods "to_json" and "from_json" to describe nodes/buildings serving as both heat and cooling suppliers.

Copy link

@marcusfuchs marcusfuchs left a comment

Choose a reason for hiding this comment

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

@jsc-tbe Thanks for the addition! I have two open questions:

  • Do we really need to introduce a new node_type with "supply_heating_cooling"? I would consider the alternative of keeping those nodes with node_type = "building". Or do we see a use case where we would have to differentiate between "building" and "supply_heating_cooling" in a network?
  • Would it be possible to add unit tests to make sure the new features work?

@TBeckhoelter
Copy link
Contributor Author

@marcusfuchs ok, I thought it would be nice to have the same keyword (supply) in the node_type of all nodes that supply heat or cold. If I change it as you suggested, there would be supply nodes with node_type = "building" but also some with node_type = "supply_heating" or node_type = "supply_cooling". For the postprocessing in uesmodel the node_type doesn't matter, the information is stored in other attributes ("is_supply_..." for supply of heat and cold and "input_heat" and "input_cool" for the demands) so I have no objections.
I just see the described classification which, from my point of view, looks a little arbitrary, as we divide three differerent types of substation into two supply types and one building without this "supply-lable".
But you are right, buildings which are part of a bidirectional network are allways supplier, there is no other type of building.
So I will change this and try to add unit tests (the week after next).

@coveralls
Copy link

coveralls commented Sep 27, 2018

Pull Request Test Coverage Report for Build 113

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at ?%

Totals Coverage Status
Change from base Build 95: 0.0%
Covered Lines:
Relevant Lines: 0

💛 - Coveralls

@TBeckhoelter
Copy link
Contributor Author

Hi @marcusfuchs, I added an unit test for the export and import methods. As you can see there are some problems with Travis CI. I just added one test function in test_uesgraph.py and an example district in example_uesgraph.py, nothing in test_visuals.py where the failure occures. I'm just comparing the imported example district with the original one. It would be nice if you could help, because I'm not so familiar with these unit tests.

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