Revision/simplify indexes #1158
Conversation
This is an attempt to remove redundancies from the c'tor signature.
Systems with storage do not build at the moment. Results cannot be processed.
Optimisation using aggregated time should now work using the tsam.TimeSeriesAggregation object as the time index. To be adjusted (not working): * GenericStorage * result processing * ...
The new name reflects what it is. (The name "invest" was often read if it were the investment costs.)
These periods do not exist anymore.
It doesn't make sense to adjust everything (esp. result processing) for the deletion of the old multi-period feature just to have to adjust it again when introducing the new one. Thus, there is a placeholder (one period hard-coded).
At the moment, it is single period.
First asigning processed results to a variable before accessing the relevant information allows to see the results while debugging.
In the multi-period model, it will not really be an added capacity, so the old name might just stay as well.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| * :math:`Y_{invest}(p)` | ||
|
|
||
| Binary variable for the status of the investment, if | ||
| :attr:`nonconvex` is `True`. | ||
| """ | ||
| m = self.parent_block() | ||
|
|
||
| def _investvar_bound_rule(block, i, o, p): | ||
| def _investvar_bound_rule(_, i, o, p): |
Check notice
Code scanning / CodeQL
Explicit returns mixed with implicit (fall through) returns Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 7 days ago
To fix the problem, we need to ensure that the function _investvar_bound_rule has an explicit return statement in all branches. This can be achieved by adding a return statement that returns None at the end of the function. This way, it is clear that the function returns None when none of the conditions are met.
- Add an explicit return statement at the end of the
_investvar_bound_rulefunction. - Ensure that the function returns
Nonewhen none of the conditions are met.
| @@ -193,3 +193,3 @@ | ||
| return 0, m.flows[i, o].investment.maximum[p] | ||
|
|
||
| return None | ||
| # Total capacity |
| * :math:`Y_{invest}(p)` | ||
|
|
||
| Binary variable for the status of the investment, if | ||
| :attr:`nonconvex` is `True`. | ||
| """ | ||
| m = self.parent_block() | ||
|
|
||
| def _investvar_bound_rule(block, i, o, p): | ||
| def _investvar_bound_rule(_, i, o, p): |
Check notice
Code scanning / CodeQL
Unused local variable Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 7 days ago
To fix the problem, we should remove the definition of the _investvar_bound_rule function since it is not used anywhere in the code. This will make the code cleaner and easier to maintain. The change should be made in the _create_variables method of the InvestmentFlowBlock class in the file src/oemof/solph/flows/_investment_flow_block.py.
| @@ -184,11 +184,2 @@ | ||
|
|
||
| def _investvar_bound_rule(_, i, o, p): | ||
| """Rule definition for bounds of invest variable.""" | ||
| if (i, o) in self.CONVEX_INVESTFLOWS: | ||
| return ( | ||
| m.flows[i, o].investment.minimum[p], | ||
| m.flows[i, o].investment.maximum[p], | ||
| ) | ||
| elif (i, o) in self.NON_CONVEX_INVESTFLOWS: | ||
| return 0, m.flows[i, o].investment.maximum[p] | ||
|
|
Processing expects the key "scalars" to be set. There is no reason we have to break the API here.
Optimises but results processing does not work, yet. (It's also unclear if results are correct.)
| energysystem.add(excess, gas_resource, wind, pv, demand, pp_gas) | ||
|
|
||
| storage = None | ||
| if True: |
Check warning
Code scanning / CodeQL
Constant in conditional expression or statement Warning
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 23 days ago
To fix the problem, we need to remove the redundant conditional statement if True: and directly execute the code inside the block. This will simplify the code and eliminate the unnecessary conditional check.
- Remove the line
if True:on line 231. - Unindent the code inside the
ifblock to align it with the surrounding code.
| @@ -230,21 +230,20 @@ | ||
| storage = None | ||
| if True: | ||
| # create storage object representing a battery | ||
| storage = solph.components.GenericStorage( | ||
| label="storage", | ||
| inputs={bel: solph.Flow(variable_costs=0.0001)}, | ||
| outputs={bel: solph.Flow()}, | ||
| loss_rate=0.01, | ||
| lifetime_inflow=10, | ||
| lifetime_outflow=10, | ||
| invest_relation_input_capacity=1 / 6, | ||
| invest_relation_output_capacity=1 / 6, | ||
| inflow_conversion_factor=1, | ||
| outflow_conversion_factor=0.8, | ||
| nominal_capacity=solph.Investment( | ||
| ep_costs=epc_storage, lifetime=10 | ||
| ), | ||
| ) | ||
| # create storage object representing a battery | ||
| storage = solph.components.GenericStorage( | ||
| label="storage", | ||
| inputs={bel: solph.Flow(variable_costs=0.0001)}, | ||
| outputs={bel: solph.Flow()}, | ||
| loss_rate=0.01, | ||
| lifetime_inflow=10, | ||
| lifetime_outflow=10, | ||
| invest_relation_input_capacity=1 / 6, | ||
| invest_relation_output_capacity=1 / 6, | ||
| inflow_conversion_factor=1, | ||
| outflow_conversion_factor=0.8, | ||
| nominal_capacity=solph.Investment( | ||
| ep_costs=epc_storage, lifetime=10 | ||
| ), | ||
| ) | ||
|
|
||
| energysystem.add(storage) | ||
| energysystem.add(storage) | ||
|
|
Name was present twice which caused problems.
|
|
||
| def _inter_storage_balance_rule(block, n, i): | ||
| """ | ||
| Rule definition for the storage balance of every storage n and | ||
| every timestep. | ||
| """ | ||
| ii = 0 | ||
| for p in m.PERIODS: | ||
| ii += len(m.es.tsa_parameters[p]["order"]) | ||
| for p in m.CAPACITY_PERIODS: |
Check failure
Code scanning / CodeQL
Suspicious unused loop iteration variable Error
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI about 9 hours ago
To fix the problem, we should rename the unused loop variable p to _ to indicate that it is intentionally unused. This will make the code clearer to readers and prevent any confusion about the purpose of the loop variable.
- Locate the for loop on line 1401 in the file
src/oemof/solph/components/_generic_storage.py. - Rename the loop variable
pto_to indicate that it is intentionally unused.
| @@ -1400,3 +1400,3 @@ | ||
| ii = 0 | ||
| for p in m.CAPACITY_PERIODS: | ||
| for _ in m.CAPACITY_PERIODS: | ||
| ii += len(m.es.tsa_parameters["order"]) |
These are no unit tests but integration test following the structure of example scripts. Collecting the examples using pytest fails, so they need to go.
The info was redundant, so the need to give it should be removed.
Casting collections.Counter to a list will lead to the keys being stored instead of the values. This eventually results in wrong tsam_weights (0 for single period).
Period/step is easier to stistinguish than inter/intra.
Simplify indexes in the TSAM compatible version (esp. removing the current multi-period investment).