Misleading/missing documentation of OffsetConverter

[lensum]

Describe the bug
In the usage documentation for the OffsetTransformer an example is given to calculate the coefficients c0 and c1. If calculated like this, the results are illogical. I narrowed it down to the calculation of c0, which should actually look like this:
c0 = (P_out_max-c1*P_in_max)/P_out_max
Which means, that c0 isn't really the y-intercept anymore (which would be c0 = P_out_max-c1*P_in_max).

Possible solutions
I see two possible solutions:

1. Fix the documentation to include the example calculations of the parameters described above, or
2. Change the calculation in the OffsetConverterBlock to not multiply with the status_nominal (which is the product of P_max(t) * Y(t)) but with the status variable. This way, c0 would remain to be the y-intercept, which, from a mathematical, logical and usage standpoint seems much nicer to me.

Am I overlooking something here or doing something wrong? I'm actually a bit surprised that this has not been noticed before.

[p-snft]

I implemented option 2.

[p-snft]

I seem to have fixed one thing but broke another, status_nominal should really be the maximum power:

Hello,` I actually disagree that this produces the right behavior, I had problems reproducing the graphs of #895 and I stumbled upon this PR.

In the documentation is it stated that P_in(p,t) = P_out(p,t) * C_1(t) + C_0(t) * P_max(p) * Y(t) and it is explicitly stated that P_max(p) * Y(t) is equivalent to status_nominal[n, o, p, t]

I noticed that the beginning of the _relation_rule loop was wrong (

oemof-solph/src/oemof/solph/components/_offset_converter.py

Line 233 in 2edeb22

expr += -m.flow[n, o, p, t]

)

It reads

expr = 0
expr += -m.flow[n, o, p, t]  # this is P_out(p,t) and it is not the one multiplied by C_1(t) !
expr += m.flow[i, n, p, t] * n.coefficients[1][t]

instead of

expr = 0
expr += -m.flow[i, n, p, t]
expr += m.flow[n, o, p, t] * n.coefficients[1][t]

I did locally reversed the changes of this PR (replacing status by nominal_status) and implemented the switch of P_in and P_out and I could reproduce the figures of #895 successfully, which was not the case with the code of this PR.

I need to look at it again with a fresh brain, after considering the points raised in #1010 as well

Originally posted by @Bachibouzouk in #1012 (comment)

[Bachibouzouk]

I used c0 and c1 as calculated in offset converter example

    # Specify the minimum and maximum loads and the corresponding efficiencies
    # for the diesel genset.
    min_load = 0.2
    max_load = 1.0
    min_efficiency = 0.20
    max_efficiency = 0.33

    # Calculate the two polynomial coefficients, i.e. the y-intersection and the
    # slope of the linear equation.
    c1 = (max_load / max_efficiency - min_load / min_efficiency) / (
        max_load - min_load
    )
    c0 = min_load * (1 / min_efficiency - c1)

With those definition, use of status_nominal and the switch of P_in and P_out in _relation_rule method, I could reproduce the original figure of the example. Maybe a solution could be to explicitly provide the way of calculating c0 and c1 in the documentation. I will double check the calculations. But from a unit perspective, the intersect should have power unit and c0 is unitless, so multiplying it with status doesn't give it a unit, whereas multiplying it with status_nominal does.

[Bachibouzouk]

The slope calculation is starting from

c1 = (P_out_max - P_out_min)/(P_in_max - P_in_min) and using P_out_i = P_in_i / efficiency_i where i can be min or max and P_in_i = P_max * load_i (where i can be min or max again). Here P_max correspond to the optimized/installed capacity of the offset converter (but the use of max here is a bit unfortunate as it increases potential confusion with other parameters P_out_max or P_in_max. By making those substitution we get

c1 = (max_load / max_efficiency - min_load / min_efficiency) / (
        max_load - min_load
    )

c0 is obtained from the green equation on the figure above (sorry I made this quickly)

c0 = P_out_min - c1 * P_in_min = P_in_min/efficiency_min - c1 * P_in_min

Then using P_in_i = P_max * load_i we get

c0/P_max = min_load * (1 / min_efficiency - c1)

Which is not the intercept anymore as @lensum pointed out, but as it gets multiplied by nominal_status then everything works out fine. It should be explicitely mentionned in the documentation though. I think it has to be provided this way as we cannot know P_max before the optimization took place and we couldn't define c0 from the min and max load and min and max efficiencies, what do you think @lensum, @p-snft ?

[p-snft]

I think one problem is that the system is overdetermined. We have the c0, c1 to describe the slope but we do also have the maximum power(s). It might make sense to let the user define the slope (my preference, the y-intercept would be a second option) and to consider the nominal_values of the flows for the creation of the constraint.

My suggestion would be: We revert #1012. This way, old models will continue behaving like they did. At the same time, we deprecate the argument coefficients and tell to use the new keyword slope instead.

[Bachibouzouk]

Reverting #1012 will not fix the error I spotted

expr = 0
expr += -m.flow[n, o, p, t]   # here it should be m.flow[i, n, p, t] instead
expr += m.flow[i, n, p, t] * n.coefficients[1][t]  # here it should be m.flow[n, o, p, t] instead

Are you sure the problem is overdetermined ? If you only know c1 (the slope) and P_max (if I am not mistaken P_max is equivalent to P_in_max, right?) how do you get C0 then?

[p-snft]

Reverting would just return the the old behaviour, which would be kept for backwards compatibility. The new implementation would use the nominal values of both flows to determine P_in_max and P_out_max.

[Bachibouzouk]

This way, old models will continue behaving like they did.

There are two things:

1. discussion about coefficients and status or status_nominal, which was raised by @lensum
2. another error which was introduced in the code of offset_converter since 61a295a and that I spotted recently

Before 61a295a, the constraint equation for the offset transformer read:

            expr = 0
            expr += -m.flow[list(n.inputs.keys())[0], n, t]
            expr += (
                m.flow[n, list(n.outputs.keys())[0], t] * n.coefficients[1][t]

Let's call it version A

And after 61a295a

It was changed to

        expr = 0
        expr += -m.flow[n, o, p, t]
        expr += (
            m.flow[i, n, p, t]
            * n.coefficients[1][t]
        )
 

Let's call it version B

So in version A the equation is correct as P_in is subtracted. In version B (which is the current version) the equation is not correct because P_out is subtracted instead of P_in. So my question is: the "old models" for which you would like to have backward compatibility are the models since version B or the ones from the time of version A? Because I am pretty sure if anyone used offset_converters since version B, the results should not make a lot of sense. I came across this because the efficiencies I computed (using v 0.5.1) with the resulting flows after optimization displayed numbers like 500k, which is nonsensical, like if energy is created within the offset_converter.

[Bachibouzouk]

The new implementation would use the nominal values of both flows to determine P_in_max and P_out_max.

Nice, I like it :)

[lensum]

I agree with the suggested changes. I initially only considered the operations optimizations where P_max_in and P_max_out are know a priori; I want to apologize for that.

So reverting back to use status_nominal is fine with me, as long it's clear how to use the component from the documentation, usage guide and the example.
If I understood the suggestion correctly, only min_load, max_load, min_efficiency and max_efficiency have to be known now, correct?

[p-snft]

Actually, I also only considered optimisations where P_max_in and P_max_out are know a priori. In that case, it should be enough to know d{P_in}/d{P_in}. If P_in and P_out are optimised, we need an additional piece of information.

So, I have to correct myself. In my (new) opinion, it makes sense to give P_out(P_in=0) and d{P_in}/d{P_in} (as in "Tuple containing the first two polynomial coefficients i.e. the y-intersection and slope of a linear equation.") in all of the cases. The documentation should be adjusted accordingly. Still, the actual formulation of the constraint needs to use status_nominal.

[upadhayaajay]

@lensum @p-snft
Noticed one more misleading in the (usage documentation for the OffsetTransformer), that the Nonconvex flow is given in inputs of the converter but in the source code it says that the Nonconvex attribute must be defined only for the output flow:
if len(self.inputs): if v.nonconvex: raise TypeError( "NonConvex attribute must be defined only for the " + "output flow!" )
I might be assuming it incorrectly, please correct me if it intended.

[p-snft]

One issue is duplicated documentation. It's wrong in usage but correct in the class reference. Maybe, we should include the parts from the class reference so that we do not forget updating the docs when changing a component.

[lensum]

I finally came around to check back into this and I can't follow your calculations @Bachibouzouk. The pivotal point is that, as far as I can see, P_out_i = P_in_i / efficiency_i should actually be P_out_i = P_in_i * efficiency_i or P_in_i = P_out_i / efficiency_i, i.e the energy leaving a conversion should be smaller than what was put inside. This leads to the following calculation (it's probably annoyingly detailled, but this way I remember what I did in a few days from now):

c_1 = (P_out_max-P_out_min)/(P_in_max-P_in_min) -> notice here that c_1 is defined for the multiplication with P_in!

using P_in = P_out/eta we get:

c_1 = (P_out_max-P_out_min)/(P_out_max/eta_max - P_out_min/eta_min)

replacing P_out_max and P_out_min with P_nom*load_max and P_nom*load_min, respectively, yields:

c_1 = (P_nom * load_max - P_nom * load_min) / (P_nom * load_max / eta_max - P_nom * load_min / eta_min)
c_1 = (load_max - load_min) / (load_max / eta_max - load_min / eta_min)

For c_0 we start from:

c_0 = P_out_min-c_1*P_in_min

Using P_in = P_out/eta again, we get:

c_0 = P_out_min - c_1*P_out_min/eta_min

and by replacing P_out_min with P_nom*load_min:

c_0 = P_nom*load_min - c_1*P_nom*load_min/eta_min
c_0/P_nom = load_min - c_1*load_min/eta_min
c_0/P_nom = load_min * (1 - c_1/eta_min)

From this, I gather the following things:

1. The equation in the NonConvexFlow using these equations should really be

expr = 0
        expr += -m.flow[n, o, p, t]
        expr += (
            m.flow[i, n, p, t]
            * n.coefficients[1][t]
        )

this is the way it was last implemented. That way, c_1 actually represents the slope of the graph. Your calculation works as well (I verified that). I think because something cancels out at some point, but c_1 is representing something else (I don't know what exactly). Or maybe I'm missing something here?

2. I used P_nom instead of P_max to reduce the potential for confusion. In this calculation, it represents the maximum capacity at the output side of the transformer. This is easily visible from the equation P_out_max = P_nom * load_max, where load_max is usually set to 1. I think this would also be benefitial in the documentation.

3. It would (imho) be better to have a function to calculate the parameters in the correct way. I, personally, find it all too confusing. Only question: where should this be located? In oemof-tools or somewhere in oemof-solph?

[p-snft]

I think, we should have an online meeting about the future structure of the OffsetConverter. (@lensum, @upadhayaajay, @oemof/oemof-solph)

[p-snft]

I had a chat with a colleague, we concluded that it would be best to give slope ("m") and abscissa ("b") but in normalised coordinates just as min/max. This way, investments would be directly scale the OffsetConverter.

[lensum]

Could you elaborate what you mean with

but in normalised coordinates just as min/max

[p-snft]

Sure. The values for min, max, and fix (of a Flow) are given in normalised coordinates, meaning that they are relative to the nominal_value (or nominal capacity) of that Flow. The coefficients could also be defined that way.

The documentation of the OffsetConverter in solph 0.5.2usage.html#offsetconverter-component reads:

calculate coefficients of input-output line equation
c0 = P_out_max - c1*P_in_max
c1 = (P_out_max-P_out_min)/(P_in_max-P_in_min)

Instead, we would have:

c1 = (1-P_out_min/P_out_max)/(1-P_in_min/P_in_max)
c0 = 1 - c1

(Now I realise that this definition means that it would be sufficient to just give one parameter because the rest of the information would be in the Flows.)

Example: A heat pump with a COP of 3 at full load would have Flow(nominal_value=heat_power_rating) for the output and Flow(nominal_value=heat_power_rating/3) for the input. With a COP of 2 at 50 % load, we have c1 = (1 - (2*0.5)/(3*1))/(1 - 0.5) = 4/3. The parameter c0 could be evaluated automatically (to be -1/3). If I wanted a time-dependent COP, I'd need to encode this into the parameter max of the Flows.

(Currently, it can happen that it has no effect if the OffsetConverter puts more restrictive limits than the Flow. Note that this design approach also means that the conversion_factors of the Converters might be dropped.)

[p-snft]

With some distance, I'd now suggest something else:

1. Keep the OffsetConverter as it is (with status_nominal) and adjust the documentation.
2. Create a FlowDefinedConverter that derives conversion factors and offset solely from the connected Flows.
3. Once the FlowDefinedConverter works, we can decide to fade out the OffsetConverter (and the Converter) or not.

[fwitte]

In trying to fix a which I thought was a bug, I ended up completely reworking the OffsetConverter. You can see the result in #1068. It includes

• The possibility of having multiple inputs and outputs at the same time.
• A fix for the examples and documentation of the OffsetConverter slope and offset specifications.
• Some renaming of parameters (see the PR)
• The user can decide on ONE flow, which has be NonConvex, which holds
  • a nominal_value,
  • the min and the max,
  • possibly also the Investment object.
• All other inputs and outputs are simple flows with no other information
• The offset and the slope are passed in normed_offsets and conversion_factors dictionaries (analogously to the standard Converter component)
• The values reference the NonConvex flow in any case.
• Two functions are available to help parametrization:
  • calculate_slope_and_offset_with_reference_to_output: You provide the max and the min of the NonConvex flow together with the efficiency (flow / reference output flow) at max and the efficiency at min. This function provides both parameters, when the reference flow is at the output of the OffsetConverter.
  • calculate_slope_and_offset_with_reference_to_input: You provide the max and the min of the NonConvex flow together with the efficiency (flow / reference input flow) at max and the efficiency at min. This function provides both parameters, when the reference flow is at the input of the OffsetConverter.