oemof / oemof-solph

A model generator for energy system modelling and optimisation (LP/MILP).
https://oemof.org
MIT License
297 stars 125 forks source link

GenericStorage not creating InvestmentFlows when used with invest_relation_input_output #1013

Open lensum opened 10 months ago

lensum commented 10 months ago

When using the GenericStorage with an investment optimization approach like this:

electric_storage = cmp.GenericStorage(
            label="electric_storage",
            inputs={el_bus: solph.Flow()},
            outputs={el_bus: solph.Flow()},
            loss_rate=0.01,  # based on 25kW for 2.5MW self consumption
            nominal_storage_capacity=solph.Investment(
                ep_costs=elstor_epc, existing=1200, maximum=2100
            ),
            invest_relation_input_capacity=0.9375,
            invest_relation_input_output=1,
            inflow_conversion_factor=0.9,
            outflow_conversion_factor=0.8 / 0.9,  # overall efficiency of 0.8
            initial_storage_level=0,
            balanced=False,
            ),
        )

I get KeyError: 'Index \'("<oemof.solph.components._generic_storage.GenericStorage: \'electric_storage\'>", "<oemof.solph.buses._bus.Bus: \'el_bus\'>", 0)\' is not valid for indexed component \'InvestmentFlowBlock.total\''

I assume that the error might be in the internal function _set_flows. I defined the self.invest_relation_input_capacity and self.invest_relation_input_output, but the _set_flows function only considers self.invest_relation_output_capacity for the output flows, meaning that my output flows don't get converted into InvestmentFlow objects. If I use invest_relation_output_capacity=0.9375 instead of the invest_relation_input_output, the model starts to solve.

p-snft commented 10 months ago

Shouldn't the Flows be created with nominal_value=Investment() to allow for investments in any case?

lensum commented 10 months ago

That might work (have to try it tomorrow) but it isn't shown like that in any of the examples. And then the _set_flows function wouldn't make much sense to me? Or is there another reason why the Investment objects in the flow should be overwritten? Maybe because of the lifetime_inflow`lifetime_outflow` attributes?

p-snft commented 10 months ago

Maybe, it makes sense to rewrite this thing anyway. I'd consider replacing the fixed relations between the investments by a limit to the c-rate. It would be an internal property that exists independent from the nominal values of the flows.

jokochems commented 10 months ago

Yeah, you are right about locating this to the _set_flows method, @lensum , I think.

p-snft commented 10 months ago

I see multiple problems with the current implementation. First of all, we create unexpected investment variables. (If the Flows do not have to have one, they are silently created.) Secondly, you can do confusing things. The invest_relation_input_capacity=0.1 will be typically read as "it needs 10 hours to charge the storage". However, you can set a "max" to the flows to bypass that:

electric_storage = cmp.GenericStorage(
    label="electric_storage",
    inputs={el_bus: solph.Flow(max=10)},  # will be charged in 1 hour
    outputs={el_bus: solph.Flow()},
    nominal_storage_capacity=solph.Investment(
        ep_costs=elstor_epc, existing=1200, maximum=2100
    ),
    invest_relation_input_capacity=0.1,
    invest_relation_input_output=0.1,
)