Introduce minimum bounds

[tiyash-basu-frequenz]

In the message common.Metric, system_bounds has now been replaced by system_bounds_min and system_bounds_max. A metric value v should now follow the following constraint:
v >= system_bounds_max.lower AND v <= system_bounds_min.lower AND v >= system_bounds_min.upper AND v <= system_bounds_max.upper

Also, two new simple RPCs have been added:

• AddMinBounds: adds a minimum bound pair, and returns the UTC timestamp until when it will stay in effect.
• AddMaxBounds: adds a maximum bound pair, and returns the UTC timestamp until when it will stay in effect.

and the RPC SetBounds has been deprecated.

[sahas-subramanian-frequenz]

Could you also mention somewhere, either in the PR or in the commit, what the (generic) motivation for this change is, and how this change solves the problem? Or linking/mentioning an issue is also fine.

What I understand is that the target power v needs to be between system_bound_max.{lower, upper}, and v needs to be outside of system_bounds_min.{lower, upper}.

I guess that makes sense if the two bounds will always overlap, with system_bounds_max will be a broader range, and we reject power requests within system_bounds_min, which I expect would lie around 0.

So we are just trying to enforce a minimum power in either direction, by setting system_bounds_min.

Is that accurate? May be worth mention in the doc-comments.

[tiyash-basu-frequenz]

Could you also mention somewhere, either in the PR or in the commit, what the (generic) motivation for this change is, and how this change solves the problem? Or linking/mentioning an issue is also fine.

I have linked the issue.

[tiyash-basu-frequenz]

Is that accurate? May be worth mention in the doc-comments.

It's correct. The details of this behaviour is already present in the documentation, as well as in the release notes. Maybe you can point me toward exactly what detail is missing in the existing docs? I'll update those then.

[sahas-subramanian-frequenz]

Maybe you can point me toward exactly what detail is missing in the existing docs?

when I was reading the docs, this was what took me the longest to figure out, that: the two bounds will always overlap, system_bounds_max will be a broader range, and we reject power requests within system_bounds_min, which I expect would lie around 0.

once I made that connection, I could guess the point of having the two bounds. So maybe just something like that - the relation between the min and the max bounds?

[tiyash-basu-frequenz]

the relation between the min and the max bounds?

I extended the existing docs to make it clearer how the min and the max bounds are supposed to be related. Please lemme know if the changes make sense.

[sahas-subramanian-frequenz]

 I extended the existing docs to make it clearer how the min and the max bounds are supposed to be related. Please lemme know if the changes make sense.

yup, I guess so. Maybe @leandro-lucarella-frequenz wants to take a look as well?

[leandro-lucarella-frequenz]

Maybe because I'm missing the original context, but I find this extremely confusing and hard to understand.

If I finally got to understand it, after reading the docs on the specs, it seems like the min bounds is a range that will be rejected and max is a range that will be accepted. If this is the case, then it looks more like inclusion/exclusion lists, but with ranges. So if they are ranges, and using set algebra, then the values will be accepted only if in max - min, right?

-    max.lower                              max.upper   +
<-------|============|--------------|==========|-------->
                  min.lower      min.upper

---- rejected
==== accepted

Does this look OK? If so, what about naming these something more like system_accept_range and system_reject_zone? I would go with asymmetric names (i.e. not both zone or range) to try to make it more clear that one narrows the range of the other. It might be something else, but I would try to go more this route than min/max which is very confusing to me.

[leandro-lucarella-frequenz]

Could you also mention somewhere, either in the PR or in the commit, what the (generic) motivation for this change is, and how this change solves the problem? Or linking/mentioning an issue is also fine.

This was also a mystery for me, I think it would be good to mention the use case for this in the bounds docs, if not at least very briefly. I mean, even after understanding the concept, I'm not sure why the reject zone is needed, what would be the use case for it.

[leandro-lucarella-frequenz]

when I was reading the docs, this was what took me the longest to figure out, that: the two bounds will always overlap, system_bounds_max will be a broader range, and we reject power requests within system_bounds_min, which I expect would lie around 0.

Maybe also include the ascii diagram I put above for extra clarity.

[tiyash-basu-frequenz]

So if they are ranges, and using set algebra, then the values will be accepted only if in max - min, right?

Correct.

what about naming these something more like system_accept_range and system_reject_zone?

How about system_inclusion_bounds and system_exclusion_bounds, to have something that's closer to existing set-theory definitions.

even after understanding the concept, I'm not sure why the reject zone is needed, what would be the use case for it.

Sure, I can add something.

Maybe also include the ascii diagram I put above for extra clarity.

Sounds like overkill, but sure.

[leandro-lucarella-frequenz]

How about system_inclusion_bounds and system_exclusion_bounds, to have something that's closer to existing set-theory definitions.

For me the bounds word here is confusing, even if at the end is represented by a Bound, for me this is a range. So system_inclusion_range and system_exclusion_range is better, but still, inclusion and exclusion IMHO is not very clear because it seems like a stretch to say a value is included or included, so IMHO accept/reject is much more accurate and clear. If you rather have both called range, is not so clear to me that using range/zone is better, so I would be fine with that too.

[leandro-lucarella-frequenz]

Sounds like overkill, but sure.

I agree it is redundant, but at least in my case, being a visual person, if I see the diagram I understand it instantly, while going through the text takes a bit more time and effort :)

[leandro-lucarella-frequenz]

Just for the records, we talked with @tiyash-basu-frequenz and agreed on system_inclusion_bounds and system_exclusion_bounds. The rationale is:

• Inclusion and exclusion because seems to be a common term in maths and statistics to convey this meaning.
• Bounds because we want to prioritize uniformity of naming (components already use the term bounds) and the extra clarity can be added in the docs.

[tiyash-basu-frequenz]

Updated. Ready for another look.

[leandro-lucarella-frequenz]

Haha, I see you made sure it is clarified everywhere 😆

For me it would be also good to put the explanation in one place and in the other places say "see the documentation for XXX".

But anyway this is good to go for me!

[tiyash-basu-frequenz]

For me it would be also good to put the explanation in one place and in the other places say "see the documentation for XXX".

I think it's better to keep it like this for now, given we are looking into extracting some common protobuf messages out into a new repo. This ensures that we do not need to copy documentation from different places, reducing chances of copy-paste errors.

I will merge the PR now.