Stream exclusion bounds from the battery pool#537
Stream exclusion bounds from the battery pool#537llucax merged 9 commits intofrequenz-floss:v0.x.xfrom
Conversation
github-actions
bot
added
part:tests
shsms
force-pushed
the
battery-pool-excl-bounds
branch
5 times, most recently
from
782ec3f to
4a49683
Compare
shsms
force-pushed
the
battery-pool-excl-bounds
branch
2 times, most recently
from
e7e3066 to
eb6e988
Compare
shsms
force-pushed
the
battery-pool-excl-bounds
branch
from
eb6e988 to
edf865d
Compare
Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
It is a class that contains lower and upper bounds, so plural is better. There are other breaking changes related to inclusion and exclusion bounds, thar are coming up in subsequent commits, so now is a nice opportunity to make this change. Signed-off-by: Sahas Subramanian <[email protected]>
`inclusion/exclusion` bounds are a better representation of the passive sign convention, than `supply/consume` bounds. But `supply/consume` bounds are not gone for ever, they will be re-implemented as unsigned `charge/discharge` bounds in a separate PR. Signed-off-by: Sahas Subramanian <[email protected]>
They were represented with `float`s before. Closes frequenz-floss#524 Signed-off-by: Sahas Subramanian <[email protected]>
This is because once we add exclusion bounds, the function will become too big. And instead, we can add it as a separate method in a subsequent commit. Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
shsms
force-pushed
the
battery-pool-excl-bounds
branch
from
edf865d to
5bb5bb3
Compare
github-actions
bot
removed
part:tooling
|
Rebased on latest |
llucax
left a comment
llucax
left a comment
There was a problem hiding this comment.
Only a few very minor comments that can be addressed in a separate PR.
| When exclusion bounds are present, they will exclude a subset of the inclusion | ||
| bounds. | ||
|
|
||
| More details [here](https://github.com/frequenz-floss/frequenz-api-common/blob/v0.3.0/proto/frequenz/api/common/metrics.proto#L37-L91). |
There was a problem hiding this comment.
This could now point to the generated docs: https://frequenz-floss.github.io/frequenz-api-common/next/python-reference/frequenz/api/common/metrics_pb2/#frequenz.api.common.metrics_pb2.Metric.system_exclusion_bounds
We could even just use python symbols for cross referencing if we add these object indexes:
diff --git a/mkdocs.yml b/mkdocs.yml
index e8ddb46..13fe0f1 100644
--- a/mkdocs.yml
+++ b/mkdocs.yml
@@ -105,7 +105,9 @@ plugins:
import:
# See https://mkdocstrings.github.io/python/usage/#import for details
- https://docs.python.org/3/objects.inv
+ - https://frequenz-floss.github.io/frequenz-api-common/v0.3/objects.inv
- https://frequenz-floss.github.io/frequenz-channels-python/v0.14/objects.inv
+ - https://frequenz-floss.github.io/frequenz-api-microgrid/v0.15/objects.inv
- https://grpc.github.io/grpc/python/objects.inv
- https://networkx.org/documentation/stable/objects.inv
- https://numpy.org/doc/stable/objects.inv| More details [here](https://github.com/frequenz-floss/frequenz-api-common/blob/v0.3.0/proto/frequenz/api/common/metrics.proto#L37-L91). | |
| See [`frequenz.api.common.metrics_pb2.Metric.system_exclusion_bounds`][] for more details. |
But for this to work we need to initialize the microgrid API generated docs and we need to tag v0.3.1 in the common API so we can have a stable link to the docs (for now only the next version is provided), so we can update this in a followup PR.
(as a nice side-effect we can remove the # pylint: disable=line-too-long too, probably :D)
There was a problem hiding this comment.
| This is the range within which power requests are NOT allowed by the battery pool. | ||
| If present, they will be a subset of the inclusion bounds. | ||
|
|
||
| More details [here](https://github.com/frequenz-floss/frequenz-api-common/blob/v0.3.0/proto/frequenz/api/common/metrics.proto#L37-L91). |
Signed-off-by: Leandro Lucarella <[email protected]>
There was a problem hiding this comment.
Luca's suggestions about documentation sound good to me.
I can only add that the release notes should include a reference or link to the documentation of inclusion/exclusion bounds, otherwise the user needs to go through the code to get the info/links. But that can be also done in a separate PR
Lets do that in a separate commit, when also improving the links in the code. |
3 participants
Based on #416, to be merged after upgrading the Microgrid API dependency to v0.15.1.
Also closes #524