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

Example on stratified thermal storage might be confusing regarding used parameters #129

Open
jakob-wo opened this issue Jun 16, 2020 · 16 comments

Comments

@jakob-wo
Copy link
Contributor

As far as I understand the stratified thermal storage facade has the parameter efficiency. In the examples I found that stratified_thermal_storage.csv holds the parameters inflow_conversion_factor and outflow_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?

@jakob-wo
Copy link
Contributor Author

I would like to use this Issue to collect some suggestions for further improvements on the examples.

  1. The example names all start with 'stratified_thermal_storage_...', four of them even start with 'stratified_thermal_storage_investment_option...'. On one hand that is superflouus because all examples are in a folder named 'stratified_thermal_storage' already. On the other hand it leads to the effect, that the long names are not displayed fully so I cannot see what the example is acually about:

Screenshot

  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. Neither do they have a header/comment in the code that explains what option_1 differs from option_2.

  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)

@c-moeller c-moeller added this to the v0.0.3 milestone Jun 17, 2020
@jakob-wo
Copy link
Contributor Author

jakob-wo commented Jun 17, 2020

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.

@jakob-wo
Copy link
Contributor Author

I will introduce the changes in PR #135.

@jakob-wo
Copy link
Contributor Author

jakob-wo commented Jun 18, 2020

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.

  1. Does that mean I cannot invest in nominal_storage_capacity and predefine the capacity (=no invest) that I need?
  2. 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.

@jakob-wo
Copy link
Contributor Author

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?

@c-moeller
Copy link
Member

I'm not sure, if this answers your questions (and even not sure, if I am completely right), but here are some thoughts:

  • the capacity is the term for the in- and output capacity ("Speicherleistung" in MW)
  • the storage capacity is the "Speicherkapazität" in MWh
  • if you set expandable=True, you leave the height of the storage open and with a given diameter a optimal storage capacity is determined

@jakob-wo
Copy link
Contributor Author

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]

@jnnr
Copy link
Member

jnnr commented Jun 18, 2020

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.

@jakob-wo
Copy link
Contributor Author

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?

@jakob-wo
Copy link
Contributor Author

@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.

@jnnr
Copy link
Member

jnnr commented Jun 22, 2020

As far as I understand the stratified thermal storage facade has the parameter efficiency. In the examples I found that stratified_thermal_storage.csv holds the parameters inflow_conversion_factor and outflow_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.

@jnnr
Copy link
Member

jnnr commented Jun 22, 2020

1. The example names all start with 'stratified_thermal_storage_...',

I agree. Let's drop the beginning 'stratified_thermal_storage_'.

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?

@jakob-wo
Copy link
Contributor Author

@jnnr: Thanks for your feedback!

  1. Regarding the example names, please have a look at PR Improvements of strat. therm. storage examples  #135. There I suggested new names last week already.
  2. Regarding the text from the docs that explains the difference: I have some questions about that we should clearify first, before we use them as descriptive comment. You find the open questions in this issue (4 days ago).

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.

  1. And there is another topic I still wonder about:

But how do you allow the nominal storage capacity to be expanded?

@jakob-wo jakob-wo modified the milestones: v0.0.3, v0.0.4 Jul 1, 2020
@jakob-wo
Copy link
Contributor Author

jakob-wo commented Sep 8, 2020

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.

@jakob-wo jakob-wo removed their assignment Sep 8, 2020
@c-moeller
Copy link
Member

@jnnr is this now fixed with you latest work? Can we close this issue?

@jnnr
Copy link
Member

jnnr commented Oct 1, 2020

The major part of it is addressed.

@c-moeller c-moeller removed this from the v0.0.4 milestone Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants