Fix and update HeatPipeine classes / oemof_heatpipe.py

oemof / DHNx

District heating system optimisation and simulation models

MIT License

30 stars 12 forks source link

Fix and update HeatPipeine classes / oemof_heatpipe.py #137

Closed joroeder closed 1 month ago

joroeder commented 10 months ago

The following issues are addressed. Sorry for putting it all in this PR. This is a complete revision of dhnx/optimization/oemof_heatpipe.py

[x] Fix oemof network 0.5.0: There were changes in oemof.network with the release 0.5.0, e.g. making function signatures explicit, that lead errors with the specific attributes of the HeatPipeline class, as the Heatpipeline class inherit from oemof.network Transformer. An update of the initiation function solves the problem, and making the arguments explicit also makes things clearer.
[x] Remove conversion factors: These were only partly implemented and do not make sense in the component. If these are needed in some cases, we could properly add them again.
[ ] Fix docstrings: The docstrings are not up to date.
[x] Fix solph 0.5.1 (periods) for existing heatpipelines

joroeder commented 10 months ago

The last commit removes the conversion factors. These were only partly implemented and do not make sense in the component. If these are needed in some cases, we could properly add them again.

joroeder commented 10 months ago

@jnettels I think it is good to solve the periods issue as you did here https://github.com/oemof/DHNx/pull/134/commits/633e9c8d4549af576fe62cefc2bf2941499132bd. So oemof.solph >= 0.5.0 can be still used.

I am still wondering where else to add the periods 😄 Therefore, I need to have a deeper look at the changes of oemof-solph. In the meanwhile, you could checkout this branch, and test if it works in your pipeline. It should work with oemof.solph >= 0.5.0 and oemof.network >= 0.5.0a4

jnettels commented 9 months ago

Thanks a lot for taking the time to fix this @joroeder. As far as I can see, your changes make the current oemof.network usable again in my test cases. Please feel free to merge. It would be nice to also merge #134 soon afterwards, but of course it could be valuable if you can spend more time to actually check it.

p-snft commented 1 month ago

The fixes for network and solph v0.5.1 overlap pretty much what I implemented in #141 (except I dropped compatibility for solph v0.5.0). Thus, I will consider this reviewed and merge it.