Open jakob-wo opened 4 years ago
I would like to use this Issue to collect some suggestions for further improvements on the examples.
If possible the name of the examples should be more speaking. Some ony differenciate by the ending option_1 and option_2 without telling what the acual difference is. Neither do they have a header/comment in the code that explains what option_1 differs from option_2.
The example stratified_thermal_storage_facade.py
does not solve the optimization model but only compares the lp-files (if I understood the code correctly!). That was very confusing to me and it took me a while to realize why I could not read and print the results. From my point of view the examples should help and guide persons that want to use the strat. therm. storage module and therefore should be as straight forward as possible.
Apart from the question if that technic is a apropiate in an example, that speciality should be highlighted and commented. Until now a comment even says 'solve':
# create and solve the optimization model
optimization_model = Model(energysystem)
optimization_model.write('storage_model_facades.lp', io_options={'symbolic_solver_labels': True})
with open('storage_model_facades.lp') as generated_file:
with open('storage_model.lp') as expected_file:
compare_lp_files(generated_file, expected_file)
I would open a PR and realize the proposed improvements, if you could give me a brief feedback on the points I mentioned above, @jnnr and @c-moeller. Especially, if there was a specific purpose that I only did not see yet or if it can be changed.
I will introduce the changes in PR #135.
In the docu it says:
There are two options to choose from: Invest into nominal_storage_capacity and capacity (charging/discharging power) with a fixed ratio. Pass invest_relation_input_capacity and either storage_capacity_cost or capacity_cost. Invest into nominal_storage_capacity and capacity independently with no fixed ratio. Pass storage_capacity_cost and capacity_cost.
nominal_storage_capacity
and predefine the capacity
(=no invest) that I need?storage_capacity_cost
AND capacity_cost
are passed. It is not clear to me whether the docu is not correct or the example is wrong.Related to my earlier question... In the API of the strat. therm. storage facade it says:
expandable (boolean) – True, if capacity can be expanded within optimization.
What do I have to select if I want to expand the nominal_storage_capacity
instead of the capacity
?
I'm not sure, if this answers your questions (and even not sure, if I am completely right), but here are some thoughts:
Ok, thaks for clearifying this.
In that case the API seems to be wrong because it claims that expandable=True
allows the capacity
to be expanded and three lines above (in the API) it defines
capacity (numeric) – Maximum production capacity [MW]
The points @c-moeller makes are correct. In case of expandable, capacity is the existing capacity. This is true for all oemof.tabular facades. If this is not well explained in the docs, we can expand it.
In case of expandable, capacity is the existing capacity.
Well, if that is the case the docu is correct and clear. But how do you allow the nominal storage capacity to be expanded?
@jnnr, could you give me a feedback on the questions/points I stated at the very beginning of this issue, please.
Especialy, if the example stratified_thermal_storage_facade.py
is not solving the model on porpuse or if it should be changed.
As far as I understand the stratified thermal storage facade has the parameter
efficiency
. In the examples I found thatstratified_thermal_storage.csv
holds the parametersinflow_conversion_factor
andoutflow_conversion factor
.
You are right. The facade has only a single parameter 'efficiency'. If the user does not use the facade (as shown in stratified_thermal_storage.py, stratified_thermal_storage_investment_option_1.py, stratified_thermal_storage_investment_option_2.py), she has to pass inflow_conversion_factor and outflow_conversion_factor, which have the same value in the examples.
To make it simpler, I propose that in the .csv we keep a single parameter, efficiency, and use it in the examples with and without facade.
1. The example names all start with 'stratified_thermal_storage_...',
I agree. Let's drop the beginning 'stratified_thermalstorage'.
1. If possible the name of the examples should be more speaking. Some ony differenciate by the ending option_1 and option_2 without telling what the acual difference is.
Proposal for renaming: stratified_thermal_storage.csv -> input_parameters.csv stratified_thermal_storage_facade.py -> facade_operation.py stratified_thermal_storage.py -> operation.py stratified_thermal_storage_investment_option_1_facade.py -> facade_investment_fixed_ratio.py stratified_thermal_storage_investment_option_1.py -> investment_fixed_ratio.py or drop? stratified_thermal_storage_investment_option_2_facade.py -> facade_free_investment.py stratified_thermal_storage_investment_option_2.py -> free_investment.py or drop?
Maybe these are too many examples? We could even drop some of the examples that do not use the facade.
Neither do they have a header/comment in the code that explains what option_1 differs from option_2.
Good idea to describe a bit more in the header. You could take the text from the docs which explains the difference.
2. The example `stratified_thermal_storage_facade.py` does not solve the optimization model but only compares the lp-files (if I understood the code correctly!). That was very confusing to me and it took me a while to realize why I could not read and print the results. From my point of view the examples should help and guide persons that want to use the strat. therm. storage module and therefore should be as straight forward as possible.
Yes. Also, to make stratified_thermal_storage_facade.py work you have to first execute stratified_thermal_storage.py. We could drop the part comparing the lp-files. Or merge it into one example?
@jnnr: Thanks for your feedback!
Does that mean I cannot invest in nominal_storage_capacity and predefine the capacity (=no invest) that I need? Apart from that I found that in one example (stratified_thermal_storage_investment_option_1_facade.py) a fixed ratio is applied (so it should be option 1) but then storage_capacity_cost AND capacity_cost are passed. It is not clear to me whether the docu is not correct or the example is wrong.
But how do you allow the nominal storage capacity to be expanded?
I will withdraw my self-assignment because I won't have the time to work on this any more. A feedback/review on PR #135 and the questions above are is still pending... I think it still makes sense to answer these questions and to carry on with the PR to improve the examples.
@jnnr is this now fixed with you latest work? Can we close this issue?
The major part of it is addressed.
As far as I understand the stratified thermal storage facade has the parameter
efficiency
. In the examples I found thatstratified_thermal_storage.csv
holds the parametersinflow_conversion_factor
andoutflow_conversion factor
. To me that is confusing because I understand all of them to have the same effect (apart from: the later differenciates between input and output). Is that on purpose? Is there a use to have all three of them?